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

No-Fork provider no longer applies terraform $$ escapes #1013

Closed
mbbush opened this issue Dec 6, 2023 · 4 comments
Closed

No-Fork provider no longer applies terraform $$ escapes #1013

mbbush opened this issue Dec 6, 2023 · 4 comments
Labels
bug Something isn't working documentation is:triaged Indicates that an issue has been reviewed. stale

Comments

@mbbush
Copy link
Collaborator

mbbush commented Dec 6, 2023

What happened?

For resources where we fork a terraform CLI process to reconcile them, if there is a ${something} in a string parameter, in order to prevent terraform from interpreting the value as a terraform variable, users needed to set the crossplane spec.forProvider field to $${something}. The corresponding status.atProvider field would be ${something}. This always felt like a bug.

This syntax is only used in a handful of use cases, to execute some sort of AWS-specific logic. The two I'm familiar with are an ssoadmin InstanceAccessControlAttribute (which I'm adding in #928) and an iot TopicRule with a destination type of kafka, where it's used to extract credentials from secretsmanager. I bet there are more, too, but I didn't find any in the existing /examples directory.

Now for resources that aren't forking a terraform CLI, this behavior no longer applies, and resources with $${something} in the spec.forProvider fields get an error from AWS because the doubled $ is now being passed through to AWS and that is not valid syntax.

This is definitely an improvement, but it's a breaking behavior change that came as a surprise, and should be documented.

How can we reproduce it?

aab2f0d
I had to make this change to what was (in the fork version) a passing test in order to make it pass in the no-fork version.

@mbbush mbbush added bug Something isn't working needs:triage labels Dec 6, 2023
@jeanduplessis
Copy link
Collaborator

@mbbush you're right we should document this, update the release notes, and also update the example manifests not to make use of these. Adding it to our backlog to action.

@jeanduplessis jeanduplessis added is:triaged Indicates that an issue has been reviewed. documentation and removed needs:triage labels Dec 7, 2023
@mbbush
Copy link
Collaborator Author

mbbush commented Dec 7, 2023

For the release notes, I'd offer a recommended migration strategy of

  1. Annotate all resources with a $$ escape in their spec with crossplane.io/paused="true"
  2. Do the provider upgrade
  3. Edit the paused resources to remove the extra $
  4. Remove the crossplane.io/paused annotation.

Copy link

github-actions bot commented Apr 2, 2024

This provider repo does not have enough maintainers to address every issue. Since there has been no activity in the last 90 days it is now marked as stale. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

@github-actions github-actions bot added the stale label Apr 2, 2024
Copy link

This issue is being closed since there has been no activity for 14 days since marking it as stale. If you still need help, feel free to comment or reopen the issue!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation is:triaged Indicates that an issue has been reviewed. stale
Projects
None yet
Development

No branches or pull requests

2 participants