fix: track only user-declared tags on project and landing zone by tfelix · Pull Request #228 · meshcloud/terraform-provider-meshstack

tfelix · 2026-07-07T16:12:38Z

Problem

Resources whose tag schema contains a restricted tag with a default value could not be managed via Terraform:

meshstack_landingzone — terraform apply crashed on create with "Provider produced inconsistent result after apply".
meshstack_project — apply succeeded, but every subsequent terraform plan showed drift, trying to remove a tag the caller may not be permitted to manage.

Root cause

On the meshObject API create path, meshStack returns a tag map that is a superset of what the caller sent: it fills in an entry for every defined tag property (empty list for unset ones) and injects defaults for restricted tags (TagService.determineTags / LandingZoneMaintenanceService.applyLandingZoneTagsOnCreation / ProjectCreationService.createProjectFromModel).

tags is an Optional + Computed attribute, so its planned value is always known. Writing that superset into it violates the plugin framework's plan/apply consistency rule (the landing zone crash), and mirroring it on Read produces perpetual drift (the project case). For restricted tags the documented "just set it to the default yourself" workaround doesn't apply — the caller may lack permission to set them at all.

Fix

tags now tracks only the tags the user declares:

Create / Update persist the plan's tags instead of the API's superset.
Read reconciles the API response down to the keys already tracked in state (reconcileTrackedTags), so server-injected restricted defaults never enter tags and never surface as drift.
Import keeps the full set meshStack returns (there is no prior state to reconcile against), so a normal import still round-trips.

Tests

Added TestAccLandingZoneRestrictedDefaultTag and TestAccProjectRestrictedDefaultTag. The landing zone / project mock clients gained an opt-in InjectTagsOnCreate hook (and now return copies, like a real backend) to simulate the server-side injection. Verified the tests fail on main (landing zone: inconsistent result after apply; project: non-empty post-refresh plan) and pass with the fix.

Docs & changelog

CHANGELOG.md: v0.23.2 FIXES: entry.
Regenerated resource docs; added a one-line note to both tags descriptions clarifying only user-declared tags are managed.

🤖 Generated with Claude Code

github-actions · 2026-07-07T16:16:17Z

📊 Test Coverage

Scope	Coverage
Unit tests (mock client)	`75.0%`
Combined (unit + acceptance)	`80.6%` — ✅ acceptance passed

Uncovered functions (combined run)

github.com/meshcloud/terraform-provider-meshstack/client/api_key_permissions.go:225:							AllApiKeyPermissions					0.0%
github.com/meshcloud/terraform-provider-meshstack/client/buildingblock.go:97:								Delete							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/client.go:48:									NewApiTokenAuthorization				0.0%
github.com/meshcloud/terraform-provider-meshstack/client/internal/logging.go:27:							Debug							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/internal/logging.go:31:							Info							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/internal/logging.go:35:							Warn							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/internal/retry.go:228:								Close							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/internal/retry.go:249:								Write							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/project_group_binding.go:27:							Read							0.0%
github.com/meshcloud/terraform-provider-meshstack/client/project_group_binding.go:31:							Create							0.0%

Combined with go tool covdata merge over the unit and acceptance coverage data. The acceptance suite runs ./internal/provider against the live backend; coverage is attributed across all packages (-coverpkg=./...).

grubmeshi

General idea sounds valid, however, we need better test support and also see remarks on feature in general (on changelog entry).

tfelix · 2026-07-08T15:50:52Z

Maybe it make sense to go face 2 face over the commits because I am not familiar especially with the testing code here.

grubmeshi

Looks much better, see some more comments. Let's also huddle tomorrow I'd say!

grubmeshi · 2026-07-08T15:59:15Z

+	}
+	if !priorTags.IsNull() {
+		var tracked map[string][]string
+		resp.Diagnostics.Append(priorTags.ElementsAs(ctx, &tracked, false)...)


f: remove? why is this put into diags?

Resolved by the reconcileTrackedTags refactor — the ElementsAs conversion (and its diagnostics) now lives inside the helper, not at the call site.

grubmeshi · 2026-07-08T15:59:54Z

+// for unset ones) plus injected restricted-tag defaults — which the caller may be unable to manage.
+// Keeping only the previously tracked keys prevents those server-side additions from entering the
+// user-managed `tags` attribute and producing spurious drift on the next plan.
+func reconcileTrackedTags(priorTags, apiTags map[string][]string) map[string][]string {


I think one can move also the state access inside this helper function saving a redundant

var priorTags types.Map resp.Diagnostics.Append(req.State.GetAttribute(ctx, path.Root("metadata").AtName("tags"), &priorTags)...)

at call site, maybe?

error handling is a bit annoying though, but pass in diags as pointer (see other methods like SetFromClientDto etc.)

Done — reconcileTrackedTags(ctx, state, tagsPath, apiTags, &diags) now reads the prior tags from state itself and appends diagnostics via the pointer, so the redundant var priorTags types.Map / GetAttribute block is gone and the five call sites (incl. the BBD one) collapse to a single line. I also extracted a pure reconcileTags(tracked, apiTags) core and unit-tested it (TestReconcileTags).

grubmeshi · 2026-07-08T16:00:22Z

+		config, wsAddr := testconfig.Workspace(t)
+		// A second tag definition the workspace does not declare: the backend still returns it as an
+		// empty list, so the fix must reconcile it away instead of surfacing it as drift.
+		undeclaredTag, _, _ := testconfig.TagDefinition(t, client.MeshObjectKind.Workspace)


f: why is this not using restricted tag? not supported for workspaces?

Workspaces do support restricted tags — the backend injects restricted-tag defaults on create via WorkspaceRegistrationController.applyDefaultTags (same restrictedDefaultsAsSerialized() path as project/landing zone). This subtest deliberately covers the broader superset case instead: the API returns an empty-list entry for every defined tag property, even undeclared ones, which is the more general reconciliation the fix has to handle. Happy to add a dedicated restricted-default workspace subtest too if you'd like both paths covered explicitly.

grubmeshi · 2026-07-08T16:05:31Z

+			Descend("key")(SetString(tagKey)),
+			Descend("restricted")(SetRawExpr("true")),
+			Descend("value_type", "email")(
+				RenameKey("string"),


f: looks like instead of cumbersomely adapting an existing example tag definition config, better define a test support file instead with fits better it's purpose (probably everything here can be removed then, except setting dynmaic values such as random suffixes etc)?

in particular spotting RenameKey here is weird and that method should only be used rarely (add this in godoc for that method)!

Switched to purpose-built test-support_tag.tf / test-support_restricted-tag.tf (both declare value_type = { string = ... } directly), so the email→string RenameKey hack is gone; the builders now only set dynamic values (target_kind, randomized key, default_value). RenameKey stays only to give each block a unique label per run — needed because builders like ProjectAndWorkspace already contribute a tag definition — and I documented that as its rare, sanctioned use in the RenameKey godoc.

meshStack fills in restricted-tag defaults on creation and returns an entry for every defined tag property, so the meshObject API response is a superset of what the caller sent. Writing that superset into the Optional+Computed `tags` attribute broke plan/apply consistency: meshstack_landingzone crashed with "Provider produced inconsistent result after apply" on create, and meshstack_project drifted on every subsequent plan (trying to remove tags the caller may not manage). Create/Update now persist the plan's tags, and Read reconciles the API response down to the keys already tracked in state. On import there is no prior state, so the full set is kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Extends the project / landing zone tag fix to every affected resource (workspace, payment_method, building_block_definition) and reworks the tests to exercise the real backend instead of the mock. meshStack returns a tag superset on create: an entry for every defined tag property (empty list when unset), plus injected defaults for restricted tags on meshProject / meshLandingZone / meshBuildingBlockDefinition. Writing that superset into the Optional+Computed `tags` attribute broke plan/apply consistency and produced perpetual drift. Each resource now persists only the declared tags on create/update and reconciles the API response down to the tracked keys on read; imports keep the full set. Also reduces mock usage: the restricted-default / superset scenarios are covered by real-backend acceptance subtests, and the mock clients return copies from Read so callers can't mutate the store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- reconcileTrackedTags now reads prior tags from state and takes a *diag.Diagnostics, collapsing the five call sites to one line; extract a pure reconcileTags core and unit-test it (TestReconcileTags). - add SetBool testconfig builder and document RenameKey's rare, sanctioned use (per-run block-label uniqueness). - build tag-definition test configs from purpose-built test-support files instead of adapting the example (drops the email->string RenameKey hack). - revert the clientmock copy-on-read changes; the added tests run against the real backend only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tfelix requested a review from grubmeshi July 7, 2026 16:29

grubmeshi requested changes Jul 8, 2026

View reviewed changes

tfelix force-pushed the feature/fix-restricted-default-tags-inconsistent-state branch from 4bde1a0 to 652f0b8 Compare July 8, 2026 15:31

tfelix requested a review from grubmeshi July 8, 2026 15:49

grubmeshi approved these changes Jul 8, 2026

View reviewed changes

tfelix and others added 3 commits July 16, 2026 17:31

tfelix force-pushed the feature/fix-restricted-default-tags-inconsistent-state branch from 0de9f78 to d6d6c0c Compare July 16, 2026 15:38

Uh oh!

Conversation

tfelix commented Jul 7, 2026

Problem

Root cause

Fix

Tests

Docs & changelog

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Test Coverage

Uh oh!

grubmeshi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tfelix commented Jul 8, 2026

Uh oh!

grubmeshi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 7, 2026 •

edited

Loading