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

fix: Updating an integration errors when changing installation_id #175

Closed

Conversation

raizyr
Copy link

@raizyr raizyr commented Aug 20, 2024

Description

Fixes #174

What - Make changes to installation_id force resource replacement
Why - Changing the the installation identifier should create new resources
How - Add a PlanModifier to the schema for the installation_id attribute

Type of change

Please leave one option from the following and delete the rest:

  • Bug fix (non-breaking change which fixes an issue)

Copy link
Contributor

@Tankilevitch Tankilevitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @raizyr, Thanks for the contribution.

Just to better understand what you are trying to solve.
Currently installation_id isn't a property that allowed to be changed from backend side as it is dependent on the installation_id the current integration is running. Meaning renaming the integration installation id without modifying the integration running it will cause the the running integration to re-create the integration with the installation id its running with.

So would appreciate if you could elaborate more on what exactly you are trying to resolve

@raizyr
Copy link
Author

raizyr commented Aug 22, 2024

@Tankilevitch Ah, it does seem that we misunderstood the integration resource. Our plan was to try to differentiate the port resources that we manage via terraform from the ones created automatically by other means. So we stumbled into this bug because our reason for changing the installation_id isn't really valid without also changing the ocean integration identifier as well.

Since there is not yet? a way to mark attributes as immutable, the only option to avoid violating the declarative model of Terraform's provider design principles is to mark it as RequiresReplace. I will add a Warning or Error though to indicate that you probably don't want to be changing the installation_id.


var testAccBaseIntegrationUpdate = strings.Replace(testPortIntegrationResourceBasic, integrationIdentifier, updatedIdentifier, -1)

// var stepOneCreatedAt, stepTwoCreatedAt string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant

@raizyr
Copy link
Author

raizyr commented Oct 31, 2024

Closing this for now. There are some ways in the provider framework to mark an attribute of the Schema as a generated value, and some workarounds using plan modifiers that could work, but ultimately this is a non-standard usage since Terraform intends to manage the full lifecycle of a resource and with integrations that is not the case. Ideally, I'd like to see the integration config or mapping decoupled from the integration schema itself, that way the integration record would be generated and fully managed by the ocean integration while the terraform provider could then manage the full lifecycle of an integration's configuration/mapping.

@raizyr raizyr closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing integration installation_id results in error
2 participants