Skip to content

build/diff: strip SOPS metadata from non-Secret resources (HelmRelease etc.)#1

Merged
SebTardif merged 4 commits intomainfrom
copilot/implement-expensive-plan
Apr 27, 2026
Merged

build/diff: strip SOPS metadata from non-Secret resources (HelmRelease etc.)#1
SebTardif merged 4 commits intomainfrom
copilot/implement-expensive-plan

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 27, 2026

End goal

kustomize-controller supports encrypting any Kubernetes resource (not just Secrets)
with SOPS — for example a HelmRelease whose spec.values block contains credentials,
or a ConfigMap with sensitive data. When such a resource is committed to git it carries
a top-level .sops metadata block containing key fingerprints, recipients, and an
encrypted MAC. The controller strips this block before applying the resource to the
cluster, but the Flux CLI (flux build kustomization, flux diff kustomization) was not
doing the same.

The end goal is for flux build kustomization and flux diff kustomization to handle
SOPS-encrypted non-Secret resources correctly:

  1. No schema error — the .sops field is not part of any CRD schema; leaving it in
    the output causes flux diff (server-side apply dry-run) to fail with a field
    validation error.
  2. No metadata leakage — key fingerprints, AGE recipients, and the encrypted MAC must
    not appear in CLI output or be sent to the API server.

The root-cause analysis and all findings from the investigation are captured in
plan.md
on this branch — the single source of truth for what was discovered, what was
implemented, and what remains.


What this PR does

Delivers the core fix plus test coverage across four changes:

1. internal/build/build.go — extend maskSopsData for non-Secret resources

maskSopsData previously handled only Secret resources. An else branch is added for
every other kind: when a .sops block containing mac: ENC[…] is detected, it is
stripped via yaml.FieldClearer. The ENC[…] ciphertext field values are left intact —
they are already opaque, not plaintext.

2. internal/build/build_test.goTestMaskSopsDataNonSecret

Table-driven unit test:

  • HelmRelease with .sops block → block stripped, ENC[…] values preserved
  • HelmRelease without .sops block → resource passes through unchanged

3. cmd/flux/create_kustomization_test.go — golden-file test for --decryption-provider

Exercises flux create kustomization --decryption-provider sops --decryption-secret …
and asserts the generated spec.decryption block. Adds --interval=1m explicitly to
avoid Cobra flag-state pollution from resetCmdArgs() zeroing createArgs.interval.

4. cmd/flux/build_kustomization_test.go — full pipeline integration tests

Two new cases in TestBuildLocalKustomization that run Builder.Build() end-to-end:

  • build helmrelease with sops metadata — asserts .sops absent, ENC[…] preserved
  • build configmap with sops metadata — same for a SOPS-encrypted ConfigMap

Fixtures: cmd/flux/testdata/build-kustomization/sops-helmrelease/ and sops-configmap/.


What remains (tracked in plan.md)

  • Consider replacing ENC[…] ciphertext with **SOPS** in non-Secret output (design
    decision; ciphertext is already opaque, so this is lower priority)
  • Update flux build kustomization docs/examples to mention SOPS HelmRelease handling
  • Cloud e2e integration test in tests/integration/

Copilot AI and others added 3 commits April 26, 2026 20:52
Agent-Logs-Url: https://github.com/SebTardif/flux2/sessions/dac56475-250d-4fa6-add6-9e7559d90d2b

Co-authored-by: SebTardif <1413412+SebTardif@users.noreply.github.com>
- Strip .sops metadata from non-Secret resources (e.g. HelmRelease)
  in maskSopsData so flux build/diff kustomization does not expose
  SOPS metadata blocks for CRD objects managed via kustomize-controller
  with spec.decryption.provider=sops
- Add TestMaskSopsDataNonSecret covering HelmRelease with and without
  .sops metadata
- Fix duplicate loop left by previous partial edit in build_test.go
- Add golden-file unit test for create kustomization --decryption-provider
  sops --decryption-secret sops-age --export

Assisted-by: copilot/claude-sonnet-4

Agent-Logs-Url: https://github.com/SebTardif/flux2/sessions/1e97b1f8-2194-4033-8c76-4c23dfb62d6b

Co-authored-by: SebTardif <1413412+SebTardif@users.noreply.github.com>
@SebTardif SebTardif marked this pull request as ready for review April 27, 2026 13:42
Copilot AI review requested due to automatic review settings April 27, 2026 13:42
@SebTardif SebTardif merged commit 0e0d390 into main Apr 27, 2026
1 of 3 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates flux build kustomization / flux diff kustomization masking behavior to properly handle SOPS-encrypted non-Secret resources by stripping the top-level .sops metadata block, preventing CRD schema validation failures and avoiding SOPS metadata leakage in CLI output.

Changes:

  • Extend maskSopsData to strip .sops metadata from non-Secret resources when SOPS-encrypted content is detected.
  • Add unit + end-to-end build pipeline tests covering HelmRelease/ConfigMap SOPS metadata stripping.
  • Add a golden-file test for flux create kustomization decryption flags and corresponding testdata fixtures.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plan.md Documents investigation, approach, and remaining follow-ups.
internal/build/build.go Adds non-Secret .sops stripping logic to maskSopsData.
internal/build/build_test.go Adds unit tests for non-Secret .sops stripping behavior.
cmd/flux/create_kustomization_test.go Adds golden test for --decryption-provider/--decryption-secret.
cmd/flux/build_kustomization_test.go Adds integration test cases ensuring .sops is stripped in build output.
cmd/flux/testdata/create_kustomization/with-sops-decryption.yaml Golden output for create kustomization decryption flags.
cmd/flux/testdata/build-kustomization/sops-helmrelease/kustomization.yaml Fixture kustomization referencing a SOPS-encrypted HelmRelease.
cmd/flux/testdata/build-kustomization/sops-helmrelease/helmrelease.yaml SOPS-encrypted HelmRelease input fixture (includes .sops).
cmd/flux/testdata/build-kustomization/sops-helmrelease-result.yaml Expected build output with .sops removed.
cmd/flux/testdata/build-kustomization/sops-configmap/kustomization.yaml Fixture kustomization referencing a SOPS-encrypted ConfigMap.
cmd/flux/testdata/build-kustomization/sops-configmap/configmap.yaml SOPS-encrypted ConfigMap input fixture (includes .sops).
cmd/flux/testdata/build-kustomization/sops-configmap-result.yaml Expected build output with .sops removed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/build/build.go
Comment on lines +751 to +756
asYaml, err := res.AsYAML()
if err != nil {
return fmt.Errorf("failed to read %s %s for sops check: %w", res.GetKind(), res.GetName(), err)
}
if bytes.Contains(asYaml, []byte("sops:")) && bytes.Contains(asYaml, []byte("mac: ENC[")) {
res.PipeE(yaml.FieldClearer{Name: "sops"})
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The non-Secret branch detects SOPS by serializing the whole resource to YAML and doing substring checks. This adds per-resource overhead during build/diff and is also brittle (it depends on YAML rendering details like mac: ENC[ formatting). Consider doing a structured lookup on the root node instead (e.g., check for a top-level sops mapping and optionally sops.mac), then clear the sops field based on that, avoiding AsYAML()/bytes.Contains entirely.

Suggested change
asYaml, err := res.AsYAML()
if err != nil {
return fmt.Errorf("failed to read %s %s for sops check: %w", res.GetKind(), res.GetName(), err)
}
if bytes.Contains(asYaml, []byte("sops:")) && bytes.Contains(asYaml, []byte("mac: ENC[")) {
res.PipeE(yaml.FieldClearer{Name: "sops"})
sopsNode, err := res.Pipe(yaml.Lookup("sops"))
if err != nil {
return fmt.Errorf("failed to read %s %s sops field: %w", res.GetKind(), res.GetName(), err)
}
if sopsNode != nil && sopsNode.YNode() != nil && sopsNode.YNode().Kind == yaml.MappingNode {
macNode, err := res.Pipe(yaml.Lookup("sops", "mac"))
if err != nil {
return fmt.Errorf("failed to read %s %s sops.mac field: %w", res.GetKind(), res.GetName(), err)
}
if macNode != nil && strings.Contains(yaml.GetValue(macNode), "ENC[") {
if err := res.PipeE(yaml.FieldClearer{Name: "sops"}); err != nil {
return fmt.Errorf("failed to clear %s %s sops field: %w", res.GetKind(), res.GetName(), err)
}
}

Copilot uses AI. Check for mistakes.
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