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: template version replacement & metadata updates #58

Merged

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Aug 1, 2024

To avoid spurious template version diffs, the template resource will only create a new template version under specific circumstances.

  1. When creating the resource, a template version will be created for each in the list.
  2. Subsequent terraform applys will hash the contents of the given directories. The provider will then check if the hash belongs to a template version known of in the previous apply.
    • If the hash doesn't belong to any, a new template version will be created
    • If the hash belongs to a previous template version, but the names are different, the name will be updated.
    • If the hash belongs to a previous template version, and the name is the same (or both auto-generated names), no update will happen.

Therefore, the version name field should only be set when it's guaranteed that it will be updated whenever the contents of the supplied directory change (such as setting it to the current git commit short-hash). Most users will likely want to use the auto-generated names anyway.

Of note, is that we can determine whether or not a new template version will be created during terraform plan.

During plan, we only compare against the last known versions to handle the case where a user reverts the template to one used multiple applys prior (such when as undoing a change).
If we stored all the hashes of all previous versions, undoing a change like this would not create a new template version in the list, and would instead be a no-op, or only update the name of an older version, which is likely confusing behaviour.

Copy link
Member Author

ethanndickson commented Aug 1, 2024

@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from b194be2 to 6f8b09f Compare August 1, 2024 10:59
@ethanndickson ethanndickson marked this pull request as ready for review August 1, 2024 10:59
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from 6f8b09f to ef6b800 Compare August 1, 2024 11:08
@ethanndickson ethanndickson self-assigned this Aug 1, 2024
@ethanndickson ethanndickson force-pushed the 07-30-feat_add_all_settings_for_template_resources branch from 5c29d3c to bf81000 Compare August 1, 2024 11:30
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from ef6b800 to ffd33bb Compare August 1, 2024 11:30
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch 2 times, most recently from ef8d298 to e54b6f5 Compare August 1, 2024 14:21
@ethanndickson ethanndickson force-pushed the 07-30-feat_add_all_settings_for_template_resources branch from bf81000 to c0950ec Compare August 2, 2024 03:03
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch 5 times, most recently from ed761c2 to 1a54748 Compare August 2, 2024 13:25
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Show resolved Hide resolved
internal/provider/template_resource_test.go Outdated Show resolved Hide resolved
internal/provider/template_resource_test.go Outdated Show resolved Hide resolved
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from 1a54748 to d930a29 Compare August 6, 2024 03:45
@ethanndickson ethanndickson changed the base branch from 07-30-feat_add_all_settings_for_template_resources to main August 6, 2024 03:45
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from d930a29 to ef38461 Compare August 6, 2024 05:46
internal/provider/template_resource.go Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
internal/provider/template_resource.go Outdated Show resolved Hide resolved
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from ef38461 to b02f6a8 Compare August 6, 2024 13:59
Copy link

@spikecurtis spikecurtis left a comment

Choose a reason for hiding this comment

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

LGTM!

@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from b02f6a8 to eaab432 Compare August 7, 2024 05:54
@ethanndickson ethanndickson force-pushed the 08-01-fix_template_version_replacement_metadata_updates branch from eaab432 to cd6a681 Compare August 7, 2024 05:56
Copy link
Member Author

ethanndickson commented Aug 7, 2024

Merge activity

@ethanndickson ethanndickson merged commit 5c965e8 into main Aug 7, 2024
3 of 13 checks passed
@ethanndickson ethanndickson deleted the 08-01-fix_template_version_replacement_metadata_updates branch August 7, 2024 06:00
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.

3 participants