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: 'endless drift' when setting template path to start with '/' #700

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

TomerHeber
Copy link
Collaborator

Issue & Steps to Reproduce / Feature Request

fixes #699

Solution

Keeping the state with "/" to avoid endless drift. Ignoring the value returned by the template if only "/" was omitted.
Added an acceptance test for this use-case and updated the integration test.

pathPrefix = prefix + ".0.path"
}

path, pathOk := d.GetOk(pathPrefix)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

save the current value

}

path, pathOk := d.GetOk(pathPrefix)

if err := writeResourceDataEx(prefix, &template, d); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this updated to the new value

if err := writeResourceDataEx(prefix, &template, d); err != nil {
return fmt.Errorf("schema resource data serialization failed: %v", err)
}

// https://github.com/env0/terraform-provider-env0/issues/699 - backend removes the "/".
if pathOk && path.(string) == "/"+template.Path {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if diff is just in "/" revert to old path - this avoids a drift and keeps state intact with what the user entered.

@@ -1006,6 +1006,84 @@ func TestUnitTemplateResource(t *testing.T) {
})
})

// https://github.com/env0/terraform-provider-env0/issues/699
t.Run("path drift", func(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test fails without the fix.

Copy link
Contributor

@alonnoga alonnoga left a comment

Choose a reason for hiding this comment

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

LGTM ty for the comments

@github-actions github-actions bot added ready to merge PR approved - can be merged once the PR owner is ready and removed pending final review labels Aug 29, 2023
@TomerHeber TomerHeber merged commit 9608383 into main Aug 29, 2023
4 checks passed
@TomerHeber TomerHeber deleted the fix-path-endless-drift-#699 branch August 29, 2023 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix integration-tests provider ready to merge PR approved - can be merged once the PR owner is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: "Endless Drift" when setting template path to start with "/"
2 participants