Validate trusted_cluster name

[bernardjkim]

Closes #48309

Teleport now prevents the creation of a trusted_cluster resource if the name does not match the cluster name. Users will be shown the following error message if they attempt to create a teleport_cluster with an invalid cluster name:

trusted cluster resource name must be the same as the remote cluster name. got: "root", expected: "root.example.com"

changelog: The trusted_cluster resource name must now match the cluster name on creation

[aws-amplify-us-west-2]

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-49169.d3pp5qlev8mo18.amplifyapp.com

[hugoShaka]

This is a strongly breaking change as we had no such limitation in the past. Most of our users TC resources are created with names not matching the trusted cluster name. I don't think we can pull this one without bumping the trusted cluster resource version. This would cause a lot of toil on both users and customer support.

Have you considered other alternatives that will not break everyone's IaC? Else we can just create a new TC version with the sanity check you added.

[hugoShaka]

Also, congrats for addressing a TODO from Sasha in v2 😅

[hugoShaka]

paging @espadolini for the trusted cluster expertise

[bernardjkim]

Validating the name seemed like the simplest approach to addressing the issue, but breaking existing automation is definitely the big concern. I'll explore some alternate solutions.

[hugoShaka]

but breaking existing automation is definitely the big concern. I'll explore some alternate solutions.

If we introduce a new resource version and run the validation only if tc.version == types.V3 it should not break everyone's automation and allow us to build sane integrations.

I explored storing both the resource name and the remote cluster name in different fields, but this might also pose challenges:

• inconsistent behaviours when trying to apply resources we already renamed before doing this change
• might face indexing issues as we try to lookup resources with their name or trusted_cluster_name depending on the context

[espadolini]

Is there any automation that doesn't break horribly when a created resource gets renamed under its nose?

[hugoShaka]

Is there any automation that doesn't break horribly when a created resource gets renamed under its nose?

Sadly, terraform can import back state. Also if you just tctl create -f your whole directory, you might not notice the renaming madness. But yes, any automation not doing "fire and forget" would explode when losing its resource unique identifier. That's why we must fix this before adding operator support.

[espadolini]

I wouldn't be against just breaking compatibility for this, even without a (meaningless) resource version bump - but we'd only get to do that on a major version change, so it'd have to be v18-only - are we ok with that?

[Tener]

What is the rationale behind the renaming anyway? Maybe we can/should drop that behaviour instead?

[hugoShaka]

even without a (meaningless) resource version bump

I disagree on the version bump being meaningless: resource versions are not only here to tell us how the manifest should be parsed. We also use them to protect users against breaking changes and ensure deterministic behaviour when applying manifests. We've been doing this semi-successfully for roles to avoid breaking existing manifests.

Adding a new resource version costs us close to nothing and makes the breaking change visible to the user. This allows us to backport the new behaviour to any supported Teleport version, and we can still deprecate and remove the old renaming version in v18 or v19 if we want.

What is the rationale behind the renaming anyway? Maybe we can/should drop that behaviour instead?

Naming TC resources after the TC cluster name allows us to lookup the TC resource by:

• looking up at who signed the certificate
• doing a single backend lookup to get the TC resource
• checking if the cert was signed by the trusted cluster's CA.

Not naming the TC resource like the cluster name would require us to iterate over every trusted cluster each time to know if a cert is trusted (or build an in-memory map) and keep track of the cluster name anyway to know which CA might haver signed this.

Also, if we were to remove the renaming logic, users who wrote TC manifests whose name don't match the cluster name will create new TCs when applying again their manifest, instead of editing the current one.

[bernardjkim]

Thanks for the additional insight! It sounds like introducing a v3 resource version with name validation is the more reasonable approach. I'll update the PR with the additional changes.

[bernardjkim]

Superseded by #49789