Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions pkg/component/kubernetes/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,20 @@ func executeKubernetesOperation(ctx *component.ExecutionContext, atmosConfig *sc
case OperationApply:
// Auto-gate apply/deploy: fail fast on structurally invalid manifests
// before contacting the cluster or delivering to a provision target.
if err := validateObjectsStructural(objects); err != nil {
return nil, err
// Component-level `validate: false` opts out explicitly.
if resolveComponentValidateEnabled(info.ComponentSection) {
if err := validateObjectsStructural(objects); err != nil {
return nil, err
}
}
return deliverApply(atmosConfig, info, ctx.Flags, objects)
case OperationDelete:
return runDelete(objects)
case OperationValidate:
if !resolveComponentValidateEnabled(info.ComponentSection) {
ui.Warningf("structural validation skipped: 'validate: false' is set for this component")
return objectsToResults("skipped", objects), nil
}
return runValidate(objects, resolveValidateOptions(ctx.Flags))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
default:
return nil, fmt.Errorf("%w: %q", errUtils.ErrKubernetesUnsupportedOperation, operation)
Expand Down
47 changes: 47 additions & 0 deletions pkg/component/kubernetes/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,53 @@ func TestRunOperationApplyGateRejectsInvalidManifest(t *testing.T) {
assert.Equal(t, 1, result.ObjectsTotal)
}

func TestRunOperationApplyGateSkippedWhenValidateDisabled(t *testing.T) {
original := newKubernetesSDKClient
t.Cleanup(func() { newKubernetesSDKClient = original })

// A structurally invalid manifest (DNS-1123-invalid name — Atmos's own
// offline opinion, not a mechanical requirement of the K8s client itself, so
// it's deliverable once the auto-gate is out of the way) that would normally
// trip the auto-gate. With `validate: false` set on the component, the gate
// must be skipped entirely and delivery must proceed to the fake cluster client.
object := kubernetesObject("v1", "ConfigMap", "Bad_Name", "")
newKubernetesSDKClient = func() (*sdkClient, error) {
client, fakeClient := newFakeSDKClientWithFake(object.DeepCopy())
prependApplyDryRunReactor(fakeClient, object.DeepCopy())
return client, nil
}

result, err := runOperation(
&component.ExecutionContext{},
&schema.AtmosConfiguration{},
&schema.ConfigAndStacksInfo{ComponentSection: map[string]any{"validate": false}},
OperationApply,
[]*unstructured.Unstructured{object},
)
require.NoError(t, err)
assert.Equal(t, 1, result.ObjectsTotal)
}

func TestRunOperationValidateSkippedWhenValidateDisabled(t *testing.T) {
original := newKubernetesSDKClient
t.Cleanup(func() { newKubernetesSDKClient = original })
newKubernetesSDKClient = func() (*sdkClient, error) {
t.Fatal("validate: false must short-circuit before any structural or cluster check")
return nil, nil
}

objects := []*unstructured.Unstructured{kubernetesObject("v1", "ConfigMap", "", "")}
result, err := runOperation(
&component.ExecutionContext{},
&schema.AtmosConfiguration{},
&schema.ConfigAndStacksInfo{ComponentSection: map[string]any{"validate": false}},
OperationValidate,
objects,
)
require.NoError(t, err)
assert.Equal(t, map[string]int{"skipped": 1}, result.ActionCounts)
}

func TestRunOperationValidateDispatches(t *testing.T) {
original := newKubernetesSDKClient
t.Cleanup(func() { newKubernetesSDKClient = original })
Expand Down
16 changes: 5 additions & 11 deletions pkg/component/kubernetes/render.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package kubernetes

import (
"bytes"
"fmt"
"os"
"path/filepath"
Expand All @@ -13,6 +12,7 @@ import (

errUtils "github.com/cloudposse/atmos/errors"
"github.com/cloudposse/atmos/pkg/data"
"github.com/cloudposse/atmos/pkg/provisioner/target"
"github.com/cloudposse/atmos/pkg/schema"
"github.com/cloudposse/atmos/pkg/ui"
u "github.com/cloudposse/atmos/pkg/utils"
Expand Down Expand Up @@ -157,21 +157,15 @@ func writeSplitManifestFiles(outputDir string, objects []*unstructured.Unstructu
}

func multiDocumentYAML(objects []*unstructured.Unstructured) ([]byte, error) {
var buffer bytes.Buffer
for i, obj := range objects {
if i > 0 {
buffer.WriteString("---\n")
}
docs := make([][]byte, 0, len(objects))
for _, obj := range objects {
manifest, err := objectYAML(obj)
if err != nil {
return nil, err
}
buffer.Write(manifest)
if !bytes.HasSuffix(manifest, []byte("\n")) {
buffer.WriteByte('\n')
}
docs = append(docs, manifest)
}
return buffer.Bytes(), nil
return target.MergeYAMLDocuments(docs), nil
}

func objectYAML(obj *unstructured.Unstructured) ([]byte, error) {
Expand Down
40 changes: 39 additions & 1 deletion pkg/component/kubernetes/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
apivalidation "k8s.io/apimachinery/pkg/util/validation"
kustomizetypes "sigs.k8s.io/kustomize/api/types"

errUtils "github.com/cloudposse/atmos/errors"
"github.com/cloudposse/atmos/pkg/perf"
Expand Down Expand Up @@ -66,13 +67,18 @@ func validateObjectsStructural(objects []*unstructured.Unstructured) error {
// structuralErrorsForObject returns the offline validation errors for a single
// object: a present, DNS-1123-conformant metadata.name and a resolvable GVK.
// (apiVersion/kind presence is already guaranteed upstream by decodeObjects.)
// Kustomize's own config objects (see isKustomizeConfigObject) are exempt from
// the metadata.name presence requirement only — a name, if given, is still
// validated, and the GVK check remains unconditional.
func structuralErrorsForObject(index int, obj *unstructured.Unstructured) []error {
var errs []error
ref := objectRef(index, obj)

name := obj.GetName()
if name == "" {
errs = append(errs, fmt.Errorf("%s: %w", ref, errUtils.ErrKubernetesMissingMetadataName))
if !isKustomizeConfigObject(obj) {
errs = append(errs, fmt.Errorf("%s: %w", ref, errUtils.ErrKubernetesMissingMetadataName))
}
} else if msgs := apivalidation.IsDNS1123Subdomain(name); len(msgs) > 0 {
errs = append(errs, fmt.Errorf("%s: %w: %s", ref, errUtils.ErrKubernetesManifestInvalidName, strings.Join(msgs, "; ")))
}
Expand All @@ -84,6 +90,38 @@ func structuralErrorsForObject(index int, obj *unstructured.Unstructured) []erro
return errs
}

// isKustomizeConfigObject reports whether obj is one of Kustomize's own reserved
// config-object kinds (Kustomization or Component). These are matched against
// Kustomize's own exported kind/version constants (sigs.k8s.io/kustomize/api/types,
// already vendored by this repo's native kustomize provider) rather than a guessed
// string, mirroring exactly what Kustomize's own EnforceFields validation checks.
// Such objects are never submitted to the Kubernetes API — they are local build
// input consumed by the kustomize tool itself — and Kustomize does not require
// (or, historically, even permit) a metadata.name on them.
func isKustomizeConfigObject(obj *unstructured.Unstructured) bool {
apiVersion, kind := obj.GetAPIVersion(), obj.GetKind()
switch {
case apiVersion == kustomizetypes.KustomizationVersion && kind == kustomizetypes.KustomizationKind:
return true
case apiVersion == kustomizetypes.ComponentVersion && kind == kustomizetypes.ComponentKind:
return true
default:
return false
}
}

// resolveComponentValidateEnabled reports whether structural validation is
// enabled for this component. Component-level `validate: false` opts out of all
// automatic (apply/deploy auto-gate) and explicit (`atmos kubernetes validate`)
// structural checks; it does not affect --server, which validates against the
// live cluster's own API rather than Atmos's offline opinion.
func resolveComponentValidateEnabled(componentSection map[string]any) bool {
if v, ok := componentSection["validate"].(bool); ok {
return v
}
return true
}

// objectRef builds a human-readable identifier for an object in validation
// messages, falling back to a positional reference when the name is missing.
func objectRef(index int, obj *unstructured.Unstructured) string {
Expand Down
67 changes: 67 additions & 0 deletions pkg/component/kubernetes/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,73 @@ func TestValidateObjectsStructuralReportsAllFailures(t *testing.T) {
assert.ErrorContains(t, err, "missing group/version/kind")
}

func TestValidateObjectsStructuralKustomizeConfigObjectsExemptFromName(t *testing.T) {
objects := []*unstructured.Unstructured{
kubernetesObject("kustomize.config.k8s.io/v1beta1", "Kustomization", "", ""),
kubernetesObject("kustomize.config.k8s.io/v1alpha1", "Component", "", ""),
}

require.NoError(t, validateObjectsStructural(objects), "Kustomize's own config objects have no metadata.name in the real Kustomize schema")
}

func TestValidateObjectsStructuralKustomizeConfigObjectInvalidNameStillFails(t *testing.T) {
// The exemption is presence-only: a name that IS given is still validated.
objects := []*unstructured.Unstructured{
kubernetesObject("kustomize.config.k8s.io/v1alpha1", "Component", "Bad_Name", ""),
}

err := validateObjectsStructural(objects)
require.Error(t, err)
assert.ErrorContains(t, err, "not a valid DNS-1123 subdomain")
}

func TestValidateObjectsStructuralNonKustomizeObjectStillRequiresName(t *testing.T) {
// Guards against over-broad matching: a normal Kubernetes API object with no
// name must still fail, regardless of delivery target.
objects := []*unstructured.Unstructured{
kubernetesObject("apps/v1", "Deployment", "", ""),
}

err := validateObjectsStructural(objects)
require.Error(t, err)
assert.ErrorContains(t, err, "is missing metadata.name")
}

func TestIsKustomizeConfigObject(t *testing.T) {
tests := []struct {
name string
obj *unstructured.Unstructured
want bool
}{
{"Kustomization at its canonical version", kubernetesObject("kustomize.config.k8s.io/v1beta1", "Kustomization", "", ""), true},
{"Component at its canonical version", kubernetesObject("kustomize.config.k8s.io/v1alpha1", "Component", "", ""), true},
{"plain Deployment", kubernetesObject("apps/v1", "Deployment", "app", ""), false},
{
"Kustomize API group but an unrecognized kind does not match",
kubernetesObject("kustomize.config.k8s.io/v1alpha1", "SomeFutureKind", "", ""),
false,
},
{
"Kustomization kind at the wrong (non-canonical) version does not match",
kubernetesObject("kustomize.config.k8s.io/v2", "Kustomization", "", ""),
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, isKustomizeConfigObject(tt.obj))
})
}
}

func TestResolveComponentValidateEnabled(t *testing.T) {
assert.True(t, resolveComponentValidateEnabled(nil), "unset defaults to enabled")
assert.True(t, resolveComponentValidateEnabled(map[string]any{}), "unset defaults to enabled")
assert.True(t, resolveComponentValidateEnabled(map[string]any{"validate": true}))
assert.False(t, resolveComponentValidateEnabled(map[string]any{"validate": false}))
assert.True(t, resolveComponentValidateEnabled(map[string]any{"validate": "false"}), "non-bool values are ignored, defaulting to enabled")
}

func TestRunValidate(t *testing.T) {
original := newKubernetesSDKClient
t.Cleanup(func() { newKubernetesSDKClient = original })
Expand Down
8 changes: 8 additions & 0 deletions pkg/datafetcher/schema/stacks/stack-config/1.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,10 @@
},
"dependencies": {
"$ref": "#/definitions/dependencies"
},
"validate": {
"type": "boolean",
"description": "Enables structural validation of this component's manifests (default true). Set to false to opt out of both the implicit apply/deploy auto-gate and the explicit 'atmos kubernetes validate' command for this component; does not affect '--server' dry-run validation against a live cluster."
}
},
"required": []
Expand Down Expand Up @@ -1200,6 +1204,10 @@
"type": "string",
"description": "For kind 'git': destination path inside the deployment repository (supports Go templates)"
},
"split": {
"type": "boolean",
"description": "For kind 'git': true fans out one file per rendered object under 'path' (a directory); false writes 'path' as a single multi-document YAML file. When unset, inferred from whether the last segment of 'path' looks like a manifest filename (matches /\\.(ya?ml|json)$/i): a match defaults to single-file mode, otherwise the directory default is preserved."
},
"auth": {
"type": "object",
"properties": {
Expand Down
58 changes: 54 additions & 4 deletions pkg/provisioner/target/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -44,6 +45,25 @@ type config struct {
CommitMessage string
Signing string
PullRequest bool
// Split selects file-vs-directory semantics for Path: true fans out one file
// per manifest under Path (a directory); false writes Path as a single
// multi-document YAML file. nil defers to resolveSplit's extension inference.
Split *bool
}

// manifestPathRE matches a manifest-looking filename in the last path segment,
// used by resolveSplit to infer single-file mode when Split is left unset.
var manifestPathRE = regexp.MustCompile(`(?i)\.(ya?ml|json)$`)

// resolveSplit implements the Split tri-state: an explicit target-config value
// wins; otherwise the last path segment is matched against manifestPathRE — a
// match defaults to single-file mode, no match preserves the unconditional
// directory-fan-out default every existing configuration already relies on.
func resolveSplit(split *bool, path string) bool {
if split != nil {
return *split
}
return !manifestPathRE.MatchString(filepath.Base(path))
}

// repoSession bundles the resolved repository and its execution context for a
Expand Down Expand Up @@ -96,7 +116,7 @@ func (g *gitProvisioner) Deliver(ctx context.Context, in *target.DeliverInput) e
return err
}

if err := writeArtifact(resolved.Workdir, cfg.Path, &in.Artifact); err != nil {
if err := writeArtifact(resolved.Workdir, cfg.Path, &in.Artifact, resolveSplit(cfg.Split, cfg.Path)); err != nil {
return err
}

Expand Down Expand Up @@ -249,9 +269,12 @@ func commitAndPush(ctx context.Context, s *repoSession, cfg *config, artifact *t
})
}

// writeArtifact replaces the managed subtree under <workdir>/<path> with the
// artifact files, so removals propagate deterministically.
func writeArtifact(workdir, path string, artifact *target.ProvisionArtifact) error {
// writeArtifact replaces the managed path <workdir>/<path> with the artifact
// files, so removals propagate deterministically. When split is true, path is
// a directory root fanned out into one file per artifact entry (unchanged,
// historical behavior). When split is false, path is the exact output file: all
// artifact entries are merged into a single multi-document YAML file.
func writeArtifact(workdir, path string, artifact *target.ProvisionArtifact, split bool) error {
// Guard against deleting the worktree root: ValidateRepoRelativePath resolves
// root-equivalent paths ("", ".", "./", "a/..") to the worktree root, and a
// subsequent os.RemoveAll there would destroy the entire repository (including
Expand All @@ -268,6 +291,10 @@ func writeArtifact(workdir, path string, artifact *target.ProvisionArtifact) err
return fmt.Errorf("%w: clearing managed path %q: %w", errUtils.ErrGitArtifactWrite, path, err)
}

if !split {
return writeSingleArtifactFile(absPath, path, artifact)
}

for _, rel := range sortedFileKeys(artifact.Files) {
repoRel := filepath.Join(path, rel)
abs, err := atmosgit.ValidateRepoRelativePath(workdir, repoRel)
Expand All @@ -284,12 +311,35 @@ func writeArtifact(workdir, path string, artifact *target.ProvisionArtifact) err
return nil
}

// writeSingleArtifactFile merges every artifact file (in deterministic order)
// into one multi-document YAML stream and writes it to absPath (repo-relative
// path, for error messages) as a single file.
func writeSingleArtifactFile(absPath, path string, artifact *target.ProvisionArtifact) error {
keys := sortedFileKeys(artifact.Files)
docs := make([][]byte, 0, len(keys))
for _, rel := range keys {
docs = append(docs, artifact.Files[rel])
}
merged := target.MergeYAMLDocuments(docs)

if err := os.MkdirAll(filepath.Dir(absPath), dirPerm); err != nil {
return fmt.Errorf("%w: creating directory for %q: %w", errUtils.ErrGitArtifactWrite, path, err)
}
if err := os.WriteFile(absPath, merged, filePerm); err != nil {
return fmt.Errorf("%w: writing %q: %w", errUtils.ErrGitArtifactWrite, path, err)
}
return nil
}

// parseConfig extracts the git target settings from the merged target block.
func parseConfig(block map[string]any) config {
cfg := config{
Repository: stringField(block, "repository"),
Path: stringField(block, "path"),
}
if split, ok := block["split"].(bool); ok {
cfg.Split = &split
}
if auth, ok := block["auth"].(map[string]any); ok {
cfg.Identity = stringField(auth, "identity")
}
Expand Down
Loading
Loading