Fix: drift issues with variable sets #889

TomerHeber · 2024-07-08T15:30:13Z

Issue & Steps to Reproduce / Feature Request

fixes #888
fixes #894
fixes #895

Solution

Fixed conflicts between environment sets and non-environment sets.
Filter out scopes assigned implicitly.
Updated tests.
Updated documentation to warn about conflicts.

TomerHeber · 2024-07-08T15:33:15Z

client/configuration_set.go

+	Id              string `json:"id"`
+	Name            string `json:"name"`
+	Description     string `json:"description"`
+	AssignmentScope string `json:"assignmentScope"`


added assignment scope to be able to filter implicit assignments.

TomerHeber · 2024-07-08T15:33:56Z

env0/resource_environment.go

@@ -433,7 +433,7 @@ func setEnvironmentSchema(ctx context.Context, d *schema.ResourceData, environme

 	setEnvironmentConfigurationSchema(ctx, d, configurationVariables)

-	if d.Get("variable_sets") != nil {
+	if variableSets, ok := d.GetOk("variable_sets"); variableSets != nil && ok {


same as "configuration" - if this field isn't used - ignore any use-cases related to it.
this will avoid possible conflicts with explicit assignment resource.

Sorry if it's a basic question but I'm pretty new to go and terraform provider so I would like to make sure 😅 Let's say we have an environment and we assign a variable set to it (by FE or API) will we see the drift there?

No need to apologize (:
Correct - without this check we will see a drift.
(We are doing the same check for "configuration").

I see, I talked about it with @GiliFaroEnv0 and it looked weird that we won't show DD but if that's how we do it for configuration it might be the way 🤔
I will raise it with the team and update you 😄

we can't hide drifts!
the set assignment might be from the FE and not from the other resource
so this is really a drift

we are not hiding drifts...
you can either use the assignment resource or set the sets inline in the environment.
You cannot use both. Otherwise, you will have constant drifts/conflicts. (Same with configuration).

It just tests which one is used... if a resource assignments are used, it ignores anything inline in the environment.

maybe we just don't understand the code...
how are you checking if another resource is used?

It's simple (but not perfect):

if "variables-set" is set in the state, it means we use the "variables-set" inline use-case.
Otherwise, it's ignored.

TomerHeber · 2024-07-08T15:34:22Z

env0/resource_environment.go

@@ -630,7 +630,9 @@ func getEnvironmentVariableSetIdsFromApi(d *schema.ResourceData, apiClient clien

 	var environmentVariableSetIds []string
 	for _, variableSet := range environmentVariableSets {
-		environmentVariableSetIds = append(environmentVariableSetIds, variableSet.Id)
+		if variableSet.AssignmentScope == "ENVIRONMENT" {


filter is required here... to avoid getting upper scopes variables (E.g. project or org).

TomerHeber · 2024-07-08T15:34:48Z

env0/resource_variable_set_assignment.go

@@ -181,6 +187,10 @@ func resourceVariableSetAssignmentRead(ctx context.Context, d *schema.ResourceDa
 	}

 	for _, apiConfigurationSet := range apiConfigurationSets {
+		if apiConfigurationSet.AssignmentScope != assignmentSchema.Scope {
+			continue


filter to avoid getting implicit variable sets... (from other scopes)

TomerHeber · 2024-07-08T15:35:16Z

env0/resource_variable_set_assignment.go

@@ -87,6 +89,10 @@ func resourceVariableSetAssignmentUpdate(ctx context.Context, d *schema.Resource

 	// In API but not in Schema - delete.
 	for _, apiConfigurationSet := range apiConfigurationSets {
+		if apiConfigurationSet.AssignmentScope != assignmentSchema.Scope {


filter to avoid getting implicit variable sets... (from other scopes)

Yossi-kerner

Regarding the general solution here, I think we can avoid code changes and just keep the docs like we do for regular variables. In the current solution, if a user uses the API or FE to assign variable sets we will ignore it and won’t show the drift

TomerHeber · 2024-07-09T12:58:16Z

Regarding the general solution here, I think we can avoid code changes and just keep the docs like we do for regular variables. In the current solution, if a user uses the API or FE to assign variable sets we will ignore it and won’t show the drift

@Yossi-kerner - that's not going to be enough. Fixes are required here...

Yossi-kerner · 2024-07-09T13:01:33Z

Regarding the general solution here, I think we can avoid code changes and just keep the docs like we do for regular variables. In the current solution, if a user uses the API or FE to assign variable sets we will ignore it and won’t show the drift

@Yossi-kerner - that's not going to be enough. Fixes are required here...

But IIUC the fixes only hide the issue no? If there will be a drift detection we will miss that no?

TomerHeber · 2024-07-09T13:38:05Z

Regarding the general solution here, I think we can avoid code changes and just keep the docs like we do for regular variables. In the current solution, if a user uses the API or FE to assign variable sets we will ignore it and won’t show the drift

@Yossi-kerner - that's not going to be enough. Fixes are required here...

But IIUC the fixes only hide the issue no? If there will be a drift detection we will miss that no?

@Yossi-kerner

Not really...
One fix: The API will still return variable sets irrespective if they were set in the environment or not. This needs to be ignored if it was not set in the environment.
Second fix: The API to return variables returns both implicit and explicit assignments. Need to filter out implicit (inherited) assignments.

Fix: drift issues with variable sets

581a96a

TomerHeber added the pending final review label Jul 8, 2024

TomerHeber self-assigned this Jul 8, 2024

github-actions bot added fix provider api-client labels Jul 8, 2024

TomerHeber commented Jul 8, 2024

View reviewed changes

TomerHeber requested a review from Yossi-kerner July 8, 2024 15:35

Yossi-kerner reviewed Jul 9, 2024

View reviewed changes

don't ignore drift

675c46e

Yossi-kerner approved these changes Jul 11, 2024

View reviewed changes

github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Jul 11, 2024

TomerHeber merged commit 08b924d into main Jul 11, 2024
5 checks passed

TomerHeber deleted the fix-variable-sets-drifts-#888 branch July 11, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: drift issues with variable sets #889

Fix: drift issues with variable sets #889

TomerHeber commented Jul 8, 2024 •

edited

Loading

TomerHeber Jul 8, 2024

TomerHeber Jul 8, 2024

Yossi-kerner Jul 10, 2024

TomerHeber Jul 10, 2024

Yossi-kerner Jul 10, 2024

GiliFaroEnv0 Jul 10, 2024

TomerHeber Jul 10, 2024

GiliFaroEnv0 Jul 11, 2024

TomerHeber Jul 11, 2024

TomerHeber Jul 8, 2024

TomerHeber Jul 8, 2024 •

edited

Loading

TomerHeber Jul 8, 2024

Yossi-kerner left a comment

TomerHeber commented Jul 9, 2024

Yossi-kerner commented Jul 9, 2024

TomerHeber commented Jul 9, 2024

Fix: drift issues with variable sets #889

Fix: drift issues with variable sets #889

Conversation

TomerHeber commented Jul 8, 2024 • edited Loading

Issue & Steps to Reproduce / Feature Request

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomerHeber Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yossi-kerner left a comment

Choose a reason for hiding this comment

TomerHeber commented Jul 9, 2024

Yossi-kerner commented Jul 9, 2024

TomerHeber commented Jul 9, 2024

TomerHeber commented Jul 8, 2024 •

edited

Loading

TomerHeber Jul 8, 2024 •

edited

Loading