Skip to content

feat(aws): add ignore_tags to preserve customer-applied resource tags#352

Open
Lytol wants to merge 2 commits into
mainfrom
ignore-tags
Open

feat(aws): add ignore_tags to preserve customer-applied resource tags#352
Lytol wants to merge 2 commits into
mainfrom
ignore-tags

Conversation

@Lytol

@Lytol Lytol commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a per-target ignore_tags option: a flat list of exact AWS tag keys that
PTD's IaC must never add or remove on managed resources. This lets customers (or
an org-wide tagging policy) apply their own tags to resources PTD manages without
our ptd ensure reverting them.

AWS-only. Azure config is intentionally not given the field (the azure-native
provider has no equivalent, and aws:ignoreTags is provider-namespaced so it
cannot affect Azure resources).

Code Flow

  • Config: IgnoreTags []string (ignore_tags) added to AWSWorkloadConfig
    and AWSControlRoomConfig (lib/types). Exposed on the Target interface as
    IgnoreTags(); implemented on the AWS target (threaded through NewTarget),
    the Azure target returns nil. Plumbed from config in
    cmd/internal/legacy/ptd_config.go.
  • Default provider: ConfigureStackRegion (lib/pulumi/common.go) sets
    aws:ignoreTags.keys[i] as nested path config, fed from StackConfig.IgnoreTags
    in inline.go. Applies to every resource on the ambient AWS provider.
  • Explicit provider: the WorkSpaces use1 (us-east-1) provider gets
    IgnoreTags on its args when set.
  • Refresh: RefreshStack (lib/pulumi/pulumi.go) passes
    optrefresh.RunProgram(true) only when the stack has aws:ignoreTags
    configured. Required: without re-running the program, pulumi refresh reuses
    the provider config from the last up, so ignoreTags is inert on refresh
    (Use new provider version or config during refresh pulumi/pulumi#13860). The gate leaves refresh behavior unchanged for every
    workload that does not use the feature.

Rollout / adoption caveats

ignoreTags is enforced at the AWS-provider level and only takes effect once a
state-persisting ptd ensure (an apply) has registered it on the target's
provider — the run that first registers it still plans against the previous
provider ("one operation late"). Consequences, verified end-to-end on staging:

  • Clean adoption is safe. Registering ignoreTags while the target's state
    has no matching tag is a no-op for resources; afterwards, customer tags added
    out-of-band are filtered on every ptd ensure (including --refresh) — never
    added, removed, or pulled into state.
  • A tag already tracked in Pulumi state is stripped once on the registering
    apply (a prior --refresh is how such a tag gets into state). This is
    detectable: ptd ensure <target> --dry-run shows - <key> for exactly these
    targets. Handle them by clearing the key from state first (a state-only
    ensure --refresh, which does not touch AWS), then apply.
  • Fleet rollout: run ptd ensure across targets while their state is clean
    of the configured keys (use the --dry-run check to catch any that are not).
  • A key added to ignore_tags later needs another registering ptd ensure on
    then-clean state before it is protected.

See the "Adopting ignore_tags on an existing target" section in
docs/CONFIGURATION.md for the step-by-step runbook.

Category of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about

Lytol added 2 commits July 15, 2026 12:26
Add a per-target `ignore_tags` list (exact AWS tag keys) wired into the
Pulumi AWS provider's ignoreTags so customer-applied tags are not stripped
by our IaC. Requires optrefresh.RunProgram(true) on refresh (scoped to
stacks that set ignoreTags) so the setting is honored during
`ensure --refresh`; provider ignoreTags is otherwise inert on refresh
(pulumi/pulumi#13860). AWS-only.
Document that ignoreTags only takes effect after a state-persisting ensure
registers it on the provider, so adopting it on a target that already has a
matching tag in state would strip that tag on the first apply. Add the
dry-run pre-check and register-while-clean rollout procedure.
@Lytol
Lytol marked this pull request as ready for review July 16, 2026 23:10
@Lytol
Lytol requested a review from a team as a code owner July 16, 2026 23:10
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

PR Review: feat(aws): add ignore_tags to preserve customer-applied resource tags

Tasks

  • Gather context
  • Read review guidelines
  • Read changed files
  • Post review

Overall this is a clean, well-scoped implementation. The path-config approach is correct, and the RunProgram(true) gate on refresh is the right fix for pulumi/pulumi#13860. Two issues worth addressing:

Important: Stale path-config entries when keys are removed

ConfigureStackRegion only adds entries — it never removes existing aws:ignoreTags.keys[i] entries from stack config. If an operator shrinks ignore_tags from [a, b, c] to [a], the stack config retains the stale keys[1] and keys[2] entries indefinitely, so tags b and c remain ignored even after the operator removed them from the list.

lib/pulumi/common.go:99-106 — before writing the new entries, the existing aws:ignoreTags config key (or its sub-keys) should be removed so the final state exactly matches the current ignore_tags list. The simplest fix is to call stack.RemoveConfig(ctx, "aws:ignoreTags") (with Path: false) before the loop; that drops the whole blob, and the loop rewrites what's needed.

Important: Silently dropped GetAllConfig error on refresh

lib/pulumi/pulumi.go:62-65:

if allCfg, cfgErr := stack.GetAllConfig(ctx); cfgErr == nil {
    if shouldRunProgramOnRefresh(allCfg) {
        refreshOpts = append(refreshOpts, optrefresh.RunProgram(true))
    }
}

If GetAllConfig fails (transient backend error, expired credentials), the error is silently swallowed and refresh proceeds without RunProgram(true). For a stack with ignoreTags configured this silently degrades to the broken behavior described in pulumi/pulumi#13860 — ignored customer tags get pulled into state and diffed away on the next up. At minimum this should log a warning so operators see why their ignoreTags setting didn't take effect.

Fix this →


Minor observations

shouldRunProgramOnRefresh assumption (lib/pulumi/pulumi.go:41): the check looks up aws:ignoreTags (the aggregated key), relying on Pulumi's automation API returning path configs merged under their parent key in GetAllConfig. The test at pulumi_test.go:90 models this correctly, but this implicit contract between the path-config writer and the config reader is subtle enough to warrant a short comment.

ControlRoomTargetFromName passing nil for control-room ignoreTags (cmd/internal/legacy/ptd_config.go:184,201): this is fine since that proxy target is only used for cross-account credential resolution, not for deploying the control room's own Pulumi stacks. The inline comment makes this clear.

Docs runbook (docs/CONFIGURATION.md): accurate — the refresh-then-apply procedure works correctly because ConfigureStackRegion runs before RefreshStack, so the RunProgram(true) gate fires on that same invocation.

View job run

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.

2 participants