Individual SAMLv2 application_configuration resource#350
Open
geekofalltrades wants to merge 16 commits intoFusionAuth:mainfrom
Open
Individual SAMLv2 application_configuration resource#350geekofalltrades wants to merge 16 commits intoFusionAuth:mainfrom
application_configuration resource#350geekofalltrades wants to merge 16 commits intoFusionAuth:mainfrom
Conversation
This is the fusionauth API name for this field.
The claude-generated code included helpers for both JSON patch and the legacy non-specific patch. According to fusionauth docs the latter is on the deprecation path and is not as specific or powerful as the newer JSON patch and JSON Merge patch options. https://fusionauth.io/docs/apis/#the-patch-http-method
Without this check, Terraform would automatically adopt an existing configuration into its state on initial creation, which is a Terraform antipattern.
This locking was added before we knew the IDP API supported PATCH semantics. It would never have protected the IDP against multiple Terraform runs, where each run would have started its own provider server and they would not have shared this mutex.
The documentation reuses verbiage from the AWS provider's security group vs. its security group rule resources to warn against managing application configurations with both resources. * https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group * https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpc_security_group_ingress_rule
The native ignore_changes lifecycle option can be used to migrate from the inline application_configuration to the standalone resource. Ticket: EZ-7330
The tests do not consistently pass due to raciness on the PATCH method for identity-provider. This needs to be fixed by FusionAuth. Ticket: EZ-7330
geekofalltrades
changed the title
application_configuration resource
We should be able to use this to make fusionauth_idp_saml_v2 not conflict with fusionauth_idp_saml_v2_application_configuration when its application_configurations is null.
* If no application_configuration is configured, ensure that an empty object is stored back to the state, regardless of what's in FusionAuth. This suppresses diffs on idp_saml_v2 when its applicationConfiguration in FusionAuth is managed by idp_saml_v2_application_configuration in Terraform. * Perform updates with PATCH + JSON merge patch instead of PUT. If no application_configuration is configured, the JSON body contains an empty object, resulting in no changes in applicationConfiguration in FusionAuth that might be managed by separate ipd_saml_v2_application_configuration in Terraform.
There is a race condition in the FusionAuth PATCH endpoint for IdPs where patches received at the same time may not all be applied. This causes these tests to fail nondeterministically. This is a bug in the FusionAuth API that needs to be fixed upstream. Ticket: EZ-7330
Providers written in terraform-plugin-sdk can't actually support explicitly empty collections vs. null configuration, but this is ideally a case that should be supported and tested for. The provider will have to be ported to the newer terraform-plugin-framework before this is possible. Ticket: EZ-7330
If the application_configuration resource already exists and is being updated, that means it is being managed by Terraform (was previously created or has been imported). If the configuration goes missing from FusionAuth, the correct thing for Terraform to do is recreate it, not fail because it has gone missing. Ticket: EZ-7330
It is idempotent to apply a JSON patch removing a field that already doesn't exist. Ticket: EZ-7330
This type contains the information we care about in native Go types without requiring further decoding like the SAMLIdentityProviderBody type. Ticket: EZ-7330
geekofalltrades
force-pushed
the
individual-association-resource
branch
from
2ce294b to
4eccae8
Compare
|
Correction: there isn't an entra IDP type in fusionauth; rather we'd update the generic OpenID Connect one (which is how we support Entra today. |
Collaborator
|
@geekofalltrades @daethnir As discussed directly this PR is still under-review. I've tested and validated the noted short-comings. FusionAuth will assess whether these are acceptable in the mainline provider or whether they will implement API changes to remove/mitigate these limitations. I will update you both directly once that has been decided. Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request starts from AI-generated code and revises it and adds tests. Adds a
fusionauth_idp_saml_v2_application_configurationresource that uses the identity-provider PATCH endpoint to declaratively manage a singleapplicationConfigurationon a SAMLv2 IdP. This resource is more suitable for decentralized management where there are Terraform root modules for each FusionAuth application being managed, and the applications all need to be configured on a central IdP.If this approach is approved, we additionally plan to add standalone
applicationConfigurationresources for the Google and Entra IdPs.Known shortcomings:
fusionauth_idp_saml_v2resource has been updated to not manageapplicationConfigurationif itsapplication_configurationconfig is null. However, the legacy terraform-plugin-sdk this provider is written with can't distinguish between null and explicitly empty. So it is not possible to have afusionauth_idp_saml_v2with an explicitly emptyapplication_configuration.We may still make some changes subject to internal review.