-
Notifications
You must be signed in to change notification settings - Fork 2k
Support IntegrationV1 in terraform provider #61764
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
base: master
Are you sure you want to change the base?
Conversation
|
Amplify deployment status
|
d530fed to
5eac265
Compare
|
@GavinFrazar - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes. |
docs/pages/reference/infrastructure-as-code/terraform-provider/data-sources/integration.mdx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of non-blocking questions
| # only the "github" integration subkind uses credentials currently, but | ||
| # credentials is excluded intentionally so that we dont have to handle plugin credentials, which are stored separately. | ||
| - "IntegrationV1.Spec.credentials" | ||
| - "IntegrationV1.Spec.GitHub" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why must we ignore the github integration completely? can we add support for this integration, or at least document that it's not supported and track it in an issue? else we will forget about this and will tell to customers: "yes we support integrations in TF" while we're not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The github integration is complicated. You specify credentials and then auth modifies the credentials (in the spec) when you create the integration and replaces them with a "credential ref" and creates/stores plugins credentials separately.
On destroy the auth server also deletes the plugin credentials referenced by the integration.
I thought that would cause a lot of problems and it's not necessary to support github integration for our cloud discovery goals.
Maybe we could just mark credentials field as computed though? I'm not sure.
I'll try that now just to see what happens, but if it's not such a trivial thing I'd rather just document it and track it in an issue until someone asks for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ticket created: #61799
| IntegrationV1.Spec.AWSOIDC.audience: | ||
| - UseValueIn("", "aws-identity-center") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this check needed? the server doesn't validate this or do we want to restrict the type of IaC integrations manageable by TF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not needed, just nice to have because terraform will reject bad inputs at planning time
5eac265 to
275a4e4
Compare
* Also updated the audience field docs and validation error message to clarify what values are supported.
275a4e4 to
52c22e6
Compare
Changelog: Added Terraform provider support for teleport_integration resources.
Closes #61401