Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow deterministic Composite Resource Names #3120

Closed
MisterMX opened this issue Jun 2, 2022 · 14 comments
Closed

Allow deterministic Composite Resource Names #3120

MisterMX opened this issue Jun 2, 2022 · 14 comments
Labels
composition enhancement New feature or request

Comments

@MisterMX
Copy link
Contributor

MisterMX commented Jun 2, 2022

What problem are you facing?

Crossplane composite resource names are generated by the kube API server through setting generatedName to <claim-name>-. The server will append a random string as suffix to make the object's name unique:

https://github.com/crossplane/crossplane/blob/master/internal/controller/apiextensions/claim/configurator.go#L134

        if !meta.WasCreated(cp) {
		// The API server returns an available name derived from
		// generateName when we perform a dry-run create. This name is
		// likely (but not guaranteed) to be available when we create
		// the composite resource. If the API server generates a name
		// that is unavailable it will return a 500 ServerTimeout error.
		cp.SetGenerateName(fmt.Sprintf("%s-", cm.GetName()))
		return errors.Wrap(c.client.Create(ctx, cp, client.DryRunAll), errName)
	}

Because of this randomness, creating the same claim multiple times will result in different composite names which makes migration tasks and traceability very difficult.

How could Crossplane help solve your problem?

Crossplane should offer a second, deterministic name generator.

For example, the generated name could be combination of the claim's name and namespace. The folks at loft-sh/vcluster do something similar. There the name is a combination of name, namespace and cluster name:

<SERVICE_ACCOUNT>-x-<SERVICE_ACCOUNT_NAMESPACE>-x-<VIRTUAL_CLUSTER_NAME>

If the name is too long, it is trimmed:

<52 Characters of previous mentioned string>-<First 10 Characters SHA256 Hash of previous mentioned string>

To ensure backwards compatibility, the feature should be enabled via a feature flag, i.e. --enable-deterministic-composite-names.

@MisterMX MisterMX added the enhancement New feature or request label Jun 2, 2022
@negz
Copy link
Member

negz commented Jun 2, 2022

I have a vague recollection that we did used to include the claim namespace at some point, but moved away from that. Perhaps because the resulting XR names were being truncated?

@MisterMX
Copy link
Contributor Author

Some more thoughts:

  • It might be necessary to have different composite names for different XRDs. In this case the configuration would need to be part of the XRD spec instead of a CLI argument.
  • Users might have the need for a more complex name generation per claim. This could be solved by providing the user a mini go template mechanism (similar to helm).

Proposal:

Add a new field spec.composite.name.template where the user can define a helm/go text/template that is executed by the reconciler:

apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
  name: test.example.com
spec:
  composite:
    name:
      # produces something like 
      # default.test-a909b.test
      template: '{{ printf "%s.%s-%s.%s" .Claim.Namespace .Claim.Kind (sha1sum .Claim.GVK | abbrev 5) .Claim.Name  }}'

@negz
Copy link
Member

negz commented Jun 21, 2022

Can you elaborate on the problems caused by having a randomly generated XR name? Are you concerned about the actual external names of your resulting managed resources or something else?

I think of claim -> XR as somewhat like deployment -> pod. When create a deployment multiple times the pods will also get different names.

@MisterMX
Copy link
Contributor Author

MisterMX commented Jun 22, 2022

Yes, the problem is that multiple creations of the same claim with the same name leads to different resources being created.

This leads to two main issues causing lot of headache for use:

  1. Migrating resource from one cluster to another is currently very clumsy. If you miss a little detail you end up with the another database being created instead of reusing the old.
  2. Connecting resources from multiple claims is not very easy. In some cases you can select the property via labels, i.e BucketArnSelector and let the provider handle the resolution. But this is not always applicable. We have a lot of complex compositions and in many we have to create the claims sequentially:
1. Create claim A
2. Copy name of composite A
4. Past name into claim B's spec
5. Create claim B

Not only is this slow and requires a manual interaction for every claim, you cannot really use it with GitOps where you would commit all your claims and wait for ArgoCD to deploy everything.

I think of claim -> XR as somewhat like deployment -> pod. When create a deployment multiple times the pods will also get different names.

But Pods are not directly referenced by other pods and do not create resources in an external system that have a different lifecycle.

@negz
Copy link
Member

negz commented Jun 30, 2022

Migrating resource from one cluster to another is currently very clumsy. If you miss a little detail you end up with the another database being created instead of reusing the old.

Can this issue be addressed by remembering to set crossplane.io/external-name annotations on the managed resources that are derived from something deterministic (e.g. the claim name, or some formatting thereof)?

Connecting resources from multiple claims is not very easy. In some cases you can select the property via labels, i.e BucketArnSelector and let the provider handle the resolution. But this is not always applicable. We have a lot of complex compositions and in many we have to create the claims sequentially:

I think I'm almost following you here, but an example would help. It sounds like you're hoping to rely on deterministic XR names in order to allow the composed resources from one claim/XR to reference composed resources from another?

@MisterMX
Copy link
Contributor Author

MisterMX commented Jul 4, 2022

Can this issue be addressed by remembering to set crossplane.io/external-name annotations on the managed resources that are derived from something deterministic (e.g. the claim name, or some formatting thereof)?

You can. But only in cases where the external name derives from the name of the managed resource. This does not work if the external name is generated by the provider backend (such as subnets in AWS).

I think I'm almost following you here, but an example would help. It sounds like you're hoping to rely on deterministic XR names in order to allow the composed resources from one claim/XR to reference composed resources from another?

This is exactly what we want. In some cases we circumvent this by using label selectors. But in many others it doesn't. For example, creating a bucket via one composition and putting the bucket's name inside an IAM policy in another. There is no way of extracting the information automatically. You have to create the claims sequentially.

BTW: I am aware that one could patch the composed name, for example by combining claim name and namespace. But you would have to do this for every resource (which is prone to errors) and you would end up with composed and composite having different names.

@refnode
Copy link

refnode commented Jul 7, 2022

I use atm the labels

  • metadata.labels[crossplane.io/claim-namespace]
  • metadata.labels[crossplane.io/claim-name]

in the patches to generate names and tags. I don't know if I oversee something with this approach, but it works so far.
Probably this approach could be a workaround for you?

@HarrisonTotty
Copy link

This would be absolutely useful to us. We have developed a composition that creates an AWS RDS instance and would like to be able to also create a Service of type ExternalName with the same name as the claim so that our developer's code can always point to the same address (mainly <claim name>.<namespace>.svc.cluster.local).

@github-actions
Copy link

Crossplane does not currently have enough maintainers to address every issue and pull request. This issue has been automatically marked as stale because it has had no activity in the last 90 days. It will be closed in 7 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

@headyj
Copy link

headyj commented Jan 28, 2023

/fresh I'm currently facing this issue with a terraform Workspace resource, which is generating terraform workspace name based on the metadata.name or the external-name annotation of the resource. Both of them being overrided, the composite is generating duplicates each time the Workspace resource is regenerated (with a new generated name), which is obviously a problem

@github-actions github-actions bot removed the stale label Jan 28, 2023
@ltdeoliveira
Copy link

ltdeoliveira commented Mar 31, 2023

/fresh

/fresh I'm currently facing this issue with a terraform Workspace resource, which is generating terraform workspace name based on the metadata.name or the external-name annotation of the resource. Both of them being overrided, the composite is generating duplicates each time the Workspace resource is regenerated (with a new generated name), which is obviously a problem

Same here, any news on this from the maintainers?

@tcd156
Copy link

tcd156 commented Apr 18, 2023

If I create an S3 bucket, I get a random suffix.

If I try to then reference that bucket on other resources, they do not get that same suffix.

(The claim-name does work. Patching that is an alternative, but seems unnecessary.)

metadata.name gets the random suffix only on the Bucket. If I then use metadata.name on a BucketPolicy, it can't find it, because the suffix is not there.

The most egregious issue here though is the lack of documentation or configuration. If I want to create my own CRD, why of all things am I unable to configure random suffix generation?

@negz
Copy link
Member

negz commented May 3, 2023

I believe there is a problem (or problems) to be solved here, but I'm not convinced deterministic XR names are the right solution. Generally our philosophy is:

  • If you need to make sure names in the external system (i.e. the actual S3 bucket name) are deterministic, use the crossplane.io/external-name annotation. This will likely involve deriving it from XR -> MR using patches. For example as @refnode suggests, you could patch from the claim name down to an MR's external-name annotation to ensure the MR name is derived deterministically from the claim name.
  • If you need to model relationships from one Crossplane resource to another, use labels, not specific names.

There's a bunch of useful context in this issue. I'm tempted to re-open it, but I think it would be more useful to have an issue or issue(s) that lead with the problem we need to solve, not the proposed solution.

@negz
Copy link
Member

negz commented May 3, 2023

I've raised a documentation issue to track this: crossplane/docs#432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
composition enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants