fix: use unique template version names

[ethanndickson]

This fixes a bug where if a template version had it's contents updated, but the template version's name remained the same, the terraform apply would fail as the server would reject a template version name collision.

Instead, the user supplies a name_suffix field that is prepended with the number of revisions using that suffix, and a randomly generated name. The full name of the template version is available at versions.*.full_name.

This PR also fixes a bug where a configured ACL everyone: use permission could be deleted and not recovered when the template metadata changes.

[ethanndickson]

• fix: use unique template version names #54 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ethanndickson and the rest of your teammates on Graphite

[spikecurtis]

Prefix feels more intuitive to me, like myversion-2-a3458b rather than a3458b-2-myversion, that way if you sort lexicographically you get a sensible outcome.

I think it should be prefix-ordinal-randomstring if you include a prefix, and if you don't include a prefix we should submit without a name and let Coderd do the picking of a random name. The way you have it now, if you don't have this set you get something like 1_stuffy_enstein4 followed by 2_watery_tart6, which seems very strange.

[spikecurtis]

why did we change from "main" to "version-one"?

[ethanndickson]

The naming I chose originally was honestly just making it harder to follow the resource tests as I was fixing them. This just reads a bit better, and I updated the data source tests for consistency.

[spikecurtis]

are you not allowed to have multiple versions with an empty suffix? I thought empty suffix is meant to mean, "pick a name for me automatically, I don't care what."

[ethanndickson]

I've been operating under the assumption that we need something that persists across multiple configurations to be able to figure out what's a new template version, and what's an existing template version that we're updating (because, ultimately we want to avoid the spurious template version diffs).

We can't use the version ID because Terraform sets all computed attributes to Unknown before it calls Update. However, they provide you a way to UseStateForUnknown where it'll read the last value in the state file and use that instead. However, if you do this, the value gets set during the plan, and so if the directory contents change, and trigger the creation of a new template version, we'd get a new version ID. However, since the version ID was set during the plan, Terraform will complain that the value after the apply differed from what was planned. I don't see any way to turn this check off.

Since the NameSuffix is not computed, and is always specified in the configuration, we can use it as the 'primary key' here. The downside being that we can't have multiple versions with an empty suffix.

Instead we could do something like generate our own Terraform template version ID, and put UseStateForUnknown on that?
I worry if all the fields in the versions set change, Terraform won't be able to figure out what element is which, but I'd have to mess around with it and see exactly how it persists set elements. Worst case scenario, if you change everything you end up with a new version with no non-metadata changes.

[ethanndickson]

Spike and I have decided to close this fix, in favour of simply allowing users to set the full name of the template version. A future PR will add documentation telling users to not set the name attribute unless they can guarantee it will change whenever the contents of the directory change (such as when the template is backed by a git repo).