coopnorge / provider-github

Apache License 2.0
23 stars 13 forks source link

Branch name must be unique? #31

Closed RobCannon closed 6 months ago

RobCannon commented 10 months ago

Just starting to play with this resource and I am immediately seeing an issue. If try to create a repo and branch with this, it works:

apiVersion: repo.github.upbound.io/v1alpha1
kind: Repository
metadata:
  name: crossplane-test
spec:
  forProvider:
    visibility: private
    archiveOnDestroy: false
    template:
    - owner: myorg
      repository: repository-template
  providerConfigRef:
    name: my-provider
---
apiVersion: repo.github.upbound.io/v1alpha1
kind: Branch
metadata:
  name: dev
spec:
  forProvider:
    repository: crossplane-test
    repositoryRef:
      name: crossplane-test
  providerConfigRef:
    name: my-provider

But, since these resources are Cluster wide resource, the Branch name will be unique in the cluster. So, if I try to create another Branch named dev for another repo, it won't work.

Shouldn't the branchName be an attribute of the Branch resource so it does not use the resource name for the branch?

RobCannon commented 10 months ago

OK, I found an answer. I am not sure if this is the recommended way. I was wondering how to handle importing resources in Crossplane and I found out about the external-name annotation. It looks like I can use that to create a resource with an arbitrary resource name and use the annotation to name the branch what I want.

apiVersion: repo.github.upbound.io/v1alpha1
kind: Branch
metadata:
  name: crossplane-test-dev
  annotations:
    crossplane.io/external-name: dev
spec:
  forProvider:
    repository: crossplane-test
    repositoryRef:
      name: crossplane-test
  providerConfigRef:
    name: my-provider

While this works, it is not obvious, it is not documented in the Branch resource and it is overloading an annotation that is meant to be used for importing a resource.

Thoughts?

Also, I see a number of documentation issues. Should I submit a PR with the corrections or would you like an issue first?

AtzeDeVries commented 8 months ago

@RobCannon yes please create a pr on doc issues!

AtzeDeVries commented 8 months ago

This is actually an interesting issue. We dont really use this to create branch resource to create branches so i have not come across this issue. I think disconnecting the branch name from metadata.name makes sense here.

If you want you can create a PR for it, else i'll create add it to a task list