From 1b76ecb2ff28f3ccab1bdb540558993b730b25cd Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Tue, 1 Jul 2025 20:52:30 -0400 Subject: [PATCH 1/3] Add push functionality --- cmd/bundle.go | 27 +- cmd/push.go | 88 ++++ cmd/push_test.go | 367 +++++++++++++ go.mod | 14 +- go.sum | 32 +- pkg/bundler/artifact.go | 343 ++++++++++-- pkg/bundler/artifact_test.go | 844 +++++++++++++++++++++++++++++- pkg/bundler/mock_artifact.go | 9 + pkg/bundler/mock_artifact_test.go | 45 ++ pkg/bundler/shims.go | 52 +- pkg/controller/mock_controller.go | 30 ++ 11 files changed, 1751 insertions(+), 100 deletions(-) create mode 100644 cmd/push.go create mode 100644 cmd/push_test.go diff --git a/cmd/bundle.go b/cmd/bundle.go index 423ecbdb7..1a1c56cee 100644 --- a/cmd/bundle.go +++ b/cmd/bundle.go @@ -10,23 +10,24 @@ import ( // bundleCmd represents the bundle command var bundleCmd = &cobra.Command{ Use: "bundle", - Short: "Bundle blueprints into distributable artifacts", - Long: `Bundle blueprints into distributable artifacts for deployment. + Short: "Bundle blueprints into a .tar.gz archive", + Long: `Bundle your Windsor blueprints into a compressed archive for distribution. -This command packages your Windsor blueprints into compressed archives that can be -distributed and deployed to target environments. The bundling process includes: +This command packages your blueprints into a .tar.gz file that can be shared, +stored, or deployed to target environments. -- Template bundling: Packages Jsonnet templates from contexts/_template/ -- Kustomize bundling: Packages Kustomize configurations -- Metadata generation: Creates deployment metadata with build information -- Archive creation: Compresses everything into .tar.gz format +Examples: + # Bundle with automatic naming + windsor bundle -t myapp:v1.0.0 -The resulting artifacts are compatible with FluxCD OCI registries and other -deployment systems that support compressed archives. + # Bundle to specific file + windsor bundle -t myapp:v1.0.0 -o myapp-v1.0.0.tar.gz -Tag format is required as "name:version". If metadata.yaml -exists, tag values override metadata values. Tag is required if no metadata.yaml -exists or if metadata.yaml lacks name/version fields.`, + # Bundle to directory (filename auto-generated) + windsor bundle -t myapp:v1.0.0 -o ./dist/ + + # Bundle using metadata.yaml for name/version + windsor bundle`, RunE: func(cmd *cobra.Command, args []string) error { controller := cmd.Context().Value(controllerKey).(ctrl.Controller) diff --git a/cmd/push.go b/cmd/push.go new file mode 100644 index 000000000..0ae10e49b --- /dev/null +++ b/cmd/push.go @@ -0,0 +1,88 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + ctrl "github.com/windsorcli/cli/pkg/controller" +) + +// pushCmd represents the push command +var pushCmd = &cobra.Command{ + Use: "push [registry/repo:tag]", + Short: "Push blueprints to an OCI registry", + Long: `Push your Windsor blueprints to an OCI registry for sharing and deployment. + +This command packages your blueprint and pushes it to any OCI-compatible registry +like Docker Hub, GitHub Container Registry, or AWS ECR. The artifacts are compatible +with FluxCD and other GitOps tools. + +Examples: + # Push to Docker Hub + windsor push docker.io/myuser/myblueprint:v1.0.0 + + # Push to GitHub Container Registry + windsor push ghcr.io/myorg/myblueprint:v1.0.0 + + # Push using metadata.yaml for name/version + windsor push registry.example.com/blueprints`, + RunE: func(cmd *cobra.Command, args []string) error { + controller := cmd.Context().Value(controllerKey).(ctrl.Controller) + + // Initialize with requirements including bundler functionality + if err := controller.InitializeWithRequirements(ctrl.Requirements{ + CommandName: cmd.Name(), + Bundler: true, + }); err != nil { + return fmt.Errorf("failed to initialize controller: %w", err) + } + + // Resolve artifact builder from controller + artifact := controller.ResolveArtifactBuilder() + if artifact == nil { + return fmt.Errorf("artifact builder not available") + } + + // Resolve all bundlers and run them + bundlers := controller.ResolveAllBundlers() + for _, bundler := range bundlers { + if err := bundler.Bundle(artifact); err != nil { + return fmt.Errorf("bundling failed: %w", err) + } + } + + // Parse registry and tag from positional argument + if len(args) == 0 { + return fmt.Errorf("registry is required: windsor push registry/repo[:tag]") + } + + var registry, tag string + arg := args[0] + + if lastColon := strings.LastIndex(arg, ":"); lastColon > 0 && lastColon < len(arg)-1 { + // Has tag in URL format (registry/repo:tag) + registry = arg[:lastColon] + tag = arg[lastColon+1:] + } else { + // No tag in URL, registry only + registry = arg + } + + // Push the artifact to the registry + if err := artifact.Push(registry, tag); err != nil { + return fmt.Errorf("failed to push artifact: %w", err) + } + + if tag != "" { + fmt.Printf("Blueprint pushed successfully to %s:%s\n", registry, tag) + } else { + fmt.Printf("Blueprint pushed successfully to %s\n", registry) + } + return nil + }, +} + +func init() { + rootCmd.AddCommand(pushCmd) +} diff --git a/cmd/push_test.go b/cmd/push_test.go new file mode 100644 index 000000000..23fe66d19 --- /dev/null +++ b/cmd/push_test.go @@ -0,0 +1,367 @@ +package cmd + +import ( + "fmt" + "testing" + + "github.com/windsorcli/cli/pkg/bundler" + "github.com/windsorcli/cli/pkg/controller" + "github.com/windsorcli/cli/pkg/di" +) + +// ============================================================================= +// Types +// ============================================================================= + +// Extend Mocks with additional fields needed for push command tests +type PushMocks struct { + *Mocks + ArtifactBuilder *bundler.MockArtifact + TemplateBundler *bundler.MockBundler + KustomizeBundler *bundler.MockBundler +} + +// ============================================================================= +// Helpers +// ============================================================================= + +func setupPushMocks(t *testing.T) *PushMocks { + setup := func(t *testing.T) *PushMocks { + t.Helper() + opts := &SetupOptions{ + ConfigStr: ` +contexts: + default: + bundler: + enabled: true`, + } + mocks := setupMocks(t, opts) + + // Create mock artifact builder + artifactBuilder := bundler.NewMockArtifact() + artifactBuilder.InitializeFunc = func(injector di.Injector) error { return nil } + artifactBuilder.AddFileFunc = func(path string, content []byte) error { return nil } + artifactBuilder.PushFunc = func(registry string, tag string) error { return nil } + + // Create mock template bundler + templateBundler := bundler.NewMockBundler() + templateBundler.InitializeFunc = func(injector di.Injector) error { return nil } + templateBundler.BundleFunc = func(artifact bundler.Artifact) error { return nil } + + // Create mock kustomize bundler + kustomizeBundler := bundler.NewMockBundler() + kustomizeBundler.InitializeFunc = func(injector di.Injector) error { return nil } + kustomizeBundler.BundleFunc = func(artifact bundler.Artifact) error { return nil } + + // Set up controller mocks + mocks.Controller.InitializeWithRequirementsFunc = func(req controller.Requirements) error { + if req.Bundler { + return nil + } + return fmt.Errorf("bundler requirement not met") + } + + // Register bundler components in the injector + mocks.Injector.Register("artifactBuilder", artifactBuilder) + mocks.Injector.Register("templateBundler", templateBundler) + mocks.Injector.Register("kustomizeBundler", kustomizeBundler) + + return &PushMocks{ + Mocks: mocks, + ArtifactBuilder: artifactBuilder, + TemplateBundler: templateBundler, + KustomizeBundler: kustomizeBundler, + } + } + + return setup(t) +} + +// ============================================================================= +// Tests +// ============================================================================= + +func TestPushCmd(t *testing.T) { + t.Run("SuccessWithTag", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + + // When executing the push command with registry and tag + rootCmd.SetArgs([]string{"push", "registry.example.com/repo:v1.0.0"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) + + t.Run("SuccessWithoutTag", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + + // When executing the push command without tag (relies on metadata.yaml) + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) + + t.Run("ErrorMissingRegistry", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + + // When executing the push command without registry + rootCmd.SetArgs([]string{"push"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "registry is required: windsor push registry/repo[:tag]" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("ErrorInitializingController", func(t *testing.T) { + // Given a push environment with failing controller initialization + mocks := setupPushMocks(t) + mocks.Controller.InitializeWithRequirementsFunc = func(req controller.Requirements) error { + return fmt.Errorf("failed to initialize controller") + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "failed to initialize controller: failed to initialize controller" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("ErrorArtifactBuilderNotAvailable", func(t *testing.T) { + // Given a push environment with no artifact builder + mocks := setupPushMocks(t) + // Don't register the artifact builder to simulate it being unavailable + mocks.Injector.Register("artifactBuilder", nil) + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "artifact builder not available" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("ErrorTemplateBundlerFails", func(t *testing.T) { + // Given a push environment with failing template bundler + mocks := setupPushMocks(t) + mocks.TemplateBundler.BundleFunc = func(artifact bundler.Artifact) error { + return fmt.Errorf("template bundling failed") + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "bundling failed: template bundling failed" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("ErrorKustomizeBundlerFails", func(t *testing.T) { + // Given a push environment with failing kustomize bundler + mocks := setupPushMocks(t) + mocks.KustomizeBundler.BundleFunc = func(artifact bundler.Artifact) error { + return fmt.Errorf("kustomize bundling failed") + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "bundling failed: kustomize bundling failed" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("ErrorArtifactPushFails", func(t *testing.T) { + // Given a push environment with failing artifact push + mocks := setupPushMocks(t) + mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { + return fmt.Errorf("push to registry failed") + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "failed to push artifact: push to registry failed" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + + t.Run("VerifyBundlerRequirementPassed", func(t *testing.T) { + // Given a push environment that validates requirements + mocks := setupPushMocks(t) + var receivedRequirements controller.Requirements + mocks.Controller.InitializeWithRequirementsFunc = func(req controller.Requirements) error { + receivedRequirements = req + return nil + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the bundler requirement should be set + if !receivedRequirements.Bundler { + t.Error("Expected bundler requirement to be true") + } + + // And the command name should be correct + if receivedRequirements.CommandName != "push" { + t.Errorf("Expected command name to be 'push', got %s", receivedRequirements.CommandName) + } + }) + + t.Run("VerifyAllBundlersAreCalled", func(t *testing.T) { + // Given a push environment that tracks bundler calls + mocks := setupPushMocks(t) + templateBundlerCalled := false + kustomizeBundlerCalled := false + + mocks.TemplateBundler.BundleFunc = func(artifact bundler.Artifact) error { + templateBundlerCalled = true + return nil + } + mocks.KustomizeBundler.BundleFunc = func(artifact bundler.Artifact) error { + kustomizeBundlerCalled = true + return nil + } + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And both bundlers should be called + if !templateBundlerCalled { + t.Error("Expected template bundler to be called") + } + if !kustomizeBundlerCalled { + t.Error("Expected kustomize bundler to be called") + } + }) + + t.Run("VerifyCorrectParametersPassedToArtifact", func(t *testing.T) { + // Given a push environment that tracks artifact parameters + mocks := setupPushMocks(t) + var receivedRegistry, receivedTag string + + mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { + receivedRegistry = registry + receivedTag = tag + return nil + } + + // When executing the push command with registry and tag + rootCmd.SetArgs([]string{"push", "registry.example.com/myapp:v2.1.0"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And the correct parameters should be passed + if receivedRegistry != "registry.example.com/myapp" { + t.Errorf("Expected registry 'registry.example.com/myapp', got %s", receivedRegistry) + } + if receivedTag != "v2.1.0" { + t.Errorf("Expected tag 'v2.1.0', got %s", receivedTag) + } + }) + + t.Run("VerifyEmptyTagHandling", func(t *testing.T) { + // Given a push environment that tracks artifact parameters + mocks := setupPushMocks(t) + var receivedTag string + + mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { + receivedTag = tag + return nil + } + + // When executing the push command without tag (registry only) + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And an empty tag should be passed + if receivedTag != "" { + t.Errorf("Expected empty tag, got %s", receivedTag) + } + }) + + t.Run("VerifyNoBundlersHandling", func(t *testing.T) { + // Given a push environment with no bundlers + mocks := setupPushMocks(t) + // Don't register any bundlers to simulate empty list + mocks.Injector.Register("templateBundler", nil) + mocks.Injector.Register("kustomizeBundler", nil) + + // When executing the push command + rootCmd.SetArgs([]string{"push", "registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned (empty bundlers list is valid) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + }) +} diff --git a/go.mod b/go.mod index ecd837ac0..62980d093 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/fluxcd/source-controller/api v1.6.2 github.com/getsops/sops/v3 v3.10.2 github.com/goccy/go-yaml v1.18.0 + github.com/google/go-containerregistry v0.20.6 github.com/google/go-jsonnet v0.21.0 github.com/hashicorp/hcl/v2 v2.23.0 github.com/shirou/gopsutil v3.21.11+incompatible @@ -82,12 +83,16 @@ require ( github.com/cloudflare/circl v1.6.1 // indirect github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443 // indirect github.com/containerd/go-cni v1.1.12 // indirect + github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/containernetworking/cni v1.2.3 // indirect github.com/coreos/go-semver v0.3.1 // indirect github.com/cosi-project/runtime v0.10.2 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect + github.com/docker/cli v28.2.2+incompatible // indirect + github.com/docker/distribution v2.8.3+incompatible // indirect + github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-units v0.5.0 // indirect github.com/dylibso/observe-sdk/go v0.0.0-20240828172851-9145d8ad07e1 // indirect @@ -102,7 +107,7 @@ require ( github.com/gertd/go-pluralize v0.2.1 // indirect github.com/getsops/gopgagent v0.0.0-20241224165529-7044f28e491e // indirect github.com/go-jose/go-jose/v4 v4.0.5 // indirect - github.com/go-logr/logr v1.4.2 // indirect + github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.1 // indirect @@ -135,6 +140,7 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect + github.com/klauspost/compress v1.18.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/lib/pq v1.10.9 // indirect github.com/mailru/easyjson v0.9.0 // indirect @@ -148,6 +154,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect + github.com/opencontainers/image-spec v1.1.1 // indirect github.com/opencontainers/runtime-spec v1.2.1 // indirect github.com/petermattis/goid v0.0.0-20240813172612-4fcff4a6cae7 // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect @@ -167,6 +174,7 @@ require ( github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834 // indirect github.com/tetratelabs/wazero v1.9.0 // indirect github.com/urfave/cli v1.22.16 // indirect + github.com/vbatts/tar-split v0.12.1 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xhit/go-str2duration/v2 v2.1.0 // indirect github.com/yusufpapurcu/wmi v1.2.4 // indirect @@ -185,13 +193,13 @@ require ( go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/mod v0.25.0 // indirect - golang.org/x/net v0.40.0 // indirect + golang.org/x/net v0.41.0 // indirect golang.org/x/oauth2 v0.30.0 // indirect golang.org/x/sync v0.15.0 // indirect golang.org/x/term v0.32.0 // indirect golang.org/x/text v0.26.0 // indirect golang.org/x/time v0.11.0 // indirect - golang.org/x/tools v0.33.0 // indirect + golang.org/x/tools v0.34.0 // indirect google.golang.org/api v0.234.0 // indirect google.golang.org/genproto v0.0.0-20250519155744-55703ea1f237 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250519155744-55703ea1f237 // indirect diff --git a/go.sum b/go.sum index 878538f9e..e31ccbf74 100644 --- a/go.sum +++ b/go.sum @@ -139,6 +139,8 @@ github.com/containerd/continuity v0.4.5 h1:ZRoN1sXq9u7V6QoHMcVWGhOwDFqZ4B9i5H6un github.com/containerd/continuity v0.4.5/go.mod h1:/lNJvtJKUQStBzpVQ1+rasXO1LAWtUQssk28EZvJ3nE= github.com/containerd/go-cni v1.1.12 h1:wm/5VD/i255hjM4uIZjBRiEQ7y98W9ACy/mHeLi4+94= github.com/containerd/go-cni v1.1.12/go.mod h1:+jaqRBdtW5faJxj2Qwg1Of7GsV66xcvnCx4mSJtUlxU= +github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8= +github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU= github.com/containernetworking/cni v1.2.3 h1:hhOcjNVUQTnzdRJ6alC5XF+wd9mfGIUaj8FuJbEslXM= github.com/containernetworking/cni v1.2.3/go.mod h1:DuLgF+aPd3DzcTQTtp/Nvl1Kim23oFKdm2okJzBQA5M= github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4= @@ -157,10 +159,14 @@ github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/r github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= -github.com/docker/cli v28.0.4+incompatible h1:pBJSJeNd9QeIWPjRcV91RVJihd/TXB77q1ef64XEu4A= -github.com/docker/cli v28.0.4+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= -github.com/docker/docker v28.0.4+incompatible h1:JNNkBctYKurkw6FrHfKqY0nKIDf5nrbxjVBtS+cdcok= -github.com/docker/docker v28.0.4+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/cli v28.2.2+incompatible h1:qzx5BNUDFqlvyq4AHzdNB7gSyVTmU4cgsyN9SdInc1A= +github.com/docker/cli v28.2.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= +github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= +github.com/docker/docker v28.2.2+incompatible h1:CjwRSksz8Yo4+RmQ339Dp/D2tGO5JxwYeqtMOEe0LDw= +github.com/docker/docker v28.2.2+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= +github.com/docker/docker-credential-helpers v0.9.3/go.mod h1:x+4Gbw9aGmChi3qTLZj8Dfn0TD20M/fuWy0E5+WDeCo= github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c= github.com/docker/go-connections v0.5.0/go.mod h1:ov60Kzw0kKElRwhNs9UlUHAE/F9Fe6GLaXnqyDdmEXc= github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4= @@ -208,8 +214,8 @@ github.com/getsops/sops/v3 v3.10.2/go.mod h1:Dmtg1qKzFsAl+yqvMgjtnLGTC0l7RnSM6DD github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE= github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= -github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= @@ -244,6 +250,8 @@ github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcb github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-containerregistry v0.20.6 h1:cvWX87UxxLgaH76b4hIvya6Dzz9qHB31qAwjAohdSTU= +github.com/google/go-containerregistry v0.20.6/go.mod h1:T0x8MuoAoKX/873bkeSfLD2FAkwCDf9/HZgsFJ02E2Y= github.com/google/go-jsonnet v0.21.0 h1:43Bk3K4zMRP/aAZm9Po2uSEjY6ALCkYUVIcz9HLGMvA= github.com/google/go-jsonnet v0.21.0/go.mod h1:tCGAu8cpUpEZcdGMmdOu37nh8bGgqubhI5v2iSk3KJQ= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -306,6 +314,8 @@ github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRt github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= +github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -432,6 +442,8 @@ github.com/tetratelabs/wazero v1.9.0 h1:IcZ56OuxrtaEz8UYNRHBrUa9bYeX9oVY93KspZZB github.com/tetratelabs/wazero v1.9.0/go.mod h1:TSbcXCfFP0L2FGkRPxHphadXPjo1T6W+CseNNY7EkjM= github.com/urfave/cli v1.22.16 h1:MH0k6uJxdwdeWQTwhSO42Pwr4YLrNLwBtg1MRgTqPdQ= github.com/urfave/cli v1.22.16/go.mod h1:EeJR6BKodywf4zciqrdw6hpCPk68JO9z5LazXZMn5Po= +github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo= +github.com/vbatts/tar-split v0.12.1/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8= github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= @@ -508,8 +520,8 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY= -golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds= +golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw= +golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA= golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -553,8 +565,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.33.0 h1:4qz2S3zmRxbGIhDIAgjxvFutSvH5EfnsYrRBj0UI0bc= -golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI= +golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= +golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/bundler/artifact.go b/pkg/bundler/artifact.go index 18b12b76d..799d17c66 100644 --- a/pkg/bundler/artifact.go +++ b/pkg/bundler/artifact.go @@ -2,11 +2,17 @@ package bundler import ( "archive/tar" + "bytes" "fmt" + "io" "path/filepath" "strings" "time" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/stream" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/shell" ) @@ -68,24 +74,29 @@ type Artifact interface { Initialize(injector di.Injector) error AddFile(path string, content []byte) error Create(outputPath string, tag string) (string, error) + Push(registry string, tag string) error } // ============================================================================= // ArtifactBuilder Implementation // ============================================================================= -// ArtifactBuilder implements the Artifact interface for blueprint artifacts +// ArtifactBuilder implements the Artifact interface type ArtifactBuilder struct { - shims *Shims - shell shell.Shell - files map[string][]byte + files map[string][]byte + shims *Shims + shell shell.Shell + tarballPath string + metadata BlueprintMetadataInput } // ============================================================================= // Constructor // ============================================================================= -// NewArtifactBuilder creates a new ArtifactBuilder instance +// NewArtifactBuilder creates a new ArtifactBuilder instance with default configuration. +// Initializes an empty file map for storing artifact contents and sets up default shims +// for system operations. The returned builder is ready for dependency injection and file operations. func NewArtifactBuilder() *ArtifactBuilder { return &ArtifactBuilder{ shims: NewShims(), @@ -97,7 +108,10 @@ func NewArtifactBuilder() *ArtifactBuilder { // Public Methods // ============================================================================= -// Initialize initializes the ArtifactBuilder with dependency injection +// Initialize sets up the ArtifactBuilder with dependency injection and resolves required dependencies. +// Extracts the shell dependency from the injector for git operations and command execution. +// The shell is used for retrieving git provenance and builder information during metadata generation. +// Returns an error if the shell cannot be resolved from the injector. func (a *ArtifactBuilder) Initialize(injector di.Injector) error { if injector != nil { shell, ok := injector.Resolve("shell").(shell.Shell) @@ -109,23 +123,109 @@ func (a *ArtifactBuilder) Initialize(injector di.Injector) error { return nil } -// AddFile adds a file with the given path and content to the artifact +// AddFile stores a file with the specified path and content in the artifact for later packaging. +// Files are held in memory until Create() or Push() is called. The path becomes the relative +// path within the generated tar.gz archive. Multiple calls with the same path will overwrite +// the previous content. Special handling exists for "_templates/metadata.yaml" during packaging. func (a *ArtifactBuilder) AddFile(path string, content []byte) error { a.files[path] = content return nil } -// Create generates a tar.gz artifact from stored files and metadata. +// Create generates a compressed tar.gz artifact file from stored files and metadata with optional tag override. // Accepts optional tag in "name:version" format to override metadata.yaml values. // Tag takes precedence over existing metadata. If no metadata.yaml exists, tag is required. // OutputPath can be file or directory - generates filename from metadata if directory. // Creates compressed tar.gz with all files plus generated metadata.yaml at root. +// Returns the final output path of the created artifact file. func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) { + finalName, finalVersion, metadata, err := a.parseTagAndResolveMetadata(tag) + if err != nil { + return "", err + } + + finalOutputPath := a.resolveOutputPath(outputPath, finalName, finalVersion) + + err = a.createTarballToDisk(finalOutputPath, metadata) + if err != nil { + return "", err + } + + a.tarballPath = finalOutputPath + a.metadata.Name = finalName + a.metadata.Version = finalVersion + + return finalOutputPath, nil +} + +// Push creates and uploads a FluxCD-compatible OCI artifact to the specified registry entirely in-memory. +// The registry parameter should be in format "registry.example.com/namespace/name" or "registry.example.com:port/namespace/name". +// The tag parameter should be in format "name:version" to override metadata, or empty to use existing metadata. +// Creates a FluxCD-compatible OCI artifact entirely in-memory without touching the filesystem. +// Uses FluxCD-specific media types and annotations for full compatibility with FluxCD OCI repositories. +// Returns an error if registry format is invalid, tag format is incorrect, or push operation fails. +func (a *ArtifactBuilder) Push(registry string, tag string) error { + finalName, finalVersion, metadata, err := a.parseTagAndResolveMetadata(tag) + if err != nil { + return err + } + + var repoName, tagName string + if tag != "" { + parts := strings.Split(tag, ":") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return fmt.Errorf("invalid tag format: %s (expected format: name:version)", tag) + } + repoName = parts[0] + tagName = parts[1] + } else { + repoName = finalName + tagName = finalVersion + } + + fullRepo := fmt.Sprintf("%s/%s", registry, repoName) + + ref, err := name.ParseReference(fmt.Sprintf("%s:%s", fullRepo, tagName)) + if err != nil { + return fmt.Errorf("invalid repository reference: %w", err) + } + + tarballContent, err := a.createTarballInMemory(metadata) + if err != nil { + return fmt.Errorf("failed to create tarball in memory: %w", err) + } + + layer := stream.NewLayer(io.NopCloser(bytes.NewReader(tarballContent))) + + img, err := a.createFluxCDCompatibleImage(layer, repoName, tagName) + if err != nil { + return fmt.Errorf("failed to create FluxCD-compatible OCI image: %w", err) + } + + err = remote.Write(ref, img) + if err != nil { + return fmt.Errorf("failed to push artifact to registry: %w", err) + } + + return nil +} + +// ============================================================================= +// Private Methods +// ============================================================================= + +// parseTagAndResolveMetadata extracts name and version from tag parameter or metadata file and generates final metadata. +// Handles tag parsing in "name:version" format and validates both components are non-empty. +// Loads existing metadata.yaml from files if present and parses it as BlueprintMetadataInput. +// Tag parameters take precedence over metadata file values for name and version. +// Requires either tag parameters or metadata file to provide name and version. +// Returns final name, version, and complete marshaled metadata ready for embedding in artifacts. +func (a *ArtifactBuilder) parseTagAndResolveMetadata(tag string) (string, string, []byte, error) { var tagName, tagVersion string if tag != "" { parts := strings.Split(tag, ":") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", fmt.Errorf("tag must be in format 'name:version', got: %s", tag) + return "", "", nil, fmt.Errorf("tag must be in format 'name:version', got: %s", tag) } tagName = parts[0] tagVersion = parts[1] @@ -136,7 +236,7 @@ func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) if hasMetadata { if err := a.shims.YamlUnmarshal(metadataData, &input); err != nil { - return "", fmt.Errorf("failed to parse metadata.yaml: %w", err) + return "", "", nil, fmt.Errorf("failed to parse metadata.yaml: %w", err) } } @@ -151,22 +251,85 @@ func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) } if finalName == "" { - return "", fmt.Errorf("name is required: provide via tag parameter or metadata.yaml") + return "", "", nil, fmt.Errorf("name is required: provide via tag parameter or metadata.yaml") } if finalVersion == "" { - return "", fmt.Errorf("version is required: provide via tag parameter or metadata.yaml") + return "", "", nil, fmt.Errorf("version is required: provide via tag parameter or metadata.yaml") } - finalOutputPath := a.resolveOutputPath(outputPath, finalName, finalVersion) - metadata, err := a.generateMetadataWithNameVersion(input, finalName, finalVersion) if err != nil { - return "", fmt.Errorf("failed to generate metadata: %w", err) + return "", "", nil, fmt.Errorf("failed to generate metadata: %w", err) } - outputFile, err := a.shims.Create(finalOutputPath) + return finalName, finalVersion, metadata, nil +} + +// createTarballInMemory builds a compressed tar.gz archive in memory and returns the complete content as bytes. +// Creates a gzip-compressed tar archive containing all stored files plus generated metadata.yaml. +// The metadata.yaml file is always written first at the root of the archive. +// Skips any existing "_templates/metadata.yaml" file to avoid duplication. +// All files are written with 0644 permissions in the archive. +// Returns the complete archive as a byte slice for in-memory operations like OCI push. +func (a *ArtifactBuilder) createTarballInMemory(metadata []byte) ([]byte, error) { + var buf bytes.Buffer + + gzipWriter := a.shims.NewGzipWriter(&buf) + defer gzipWriter.Close() + + tarWriter := a.shims.NewTarWriter(gzipWriter) + defer tarWriter.Close() + + metadataHeader := &tar.Header{ + Name: "metadata.yaml", + Mode: 0644, + Size: int64(len(metadata)), + } + + if err := tarWriter.WriteHeader(metadataHeader); err != nil { + return nil, fmt.Errorf("failed to write metadata header: %w", err) + } + + if _, err := tarWriter.Write(metadata); err != nil { + return nil, fmt.Errorf("failed to write metadata: %w", err) + } + + for path, content := range a.files { + if path == "_templates/metadata.yaml" { + continue + } + + header := &tar.Header{ + Name: path, + Mode: 0644, + Size: int64(len(content)), + } + + if err := tarWriter.WriteHeader(header); err != nil { + return nil, fmt.Errorf("failed to write header for %s: %w", path, err) + } + + if _, err := tarWriter.Write(content); err != nil { + return nil, fmt.Errorf("failed to write content for %s: %w", path, err) + } + } + + tarWriter.Close() + gzipWriter.Close() + + return buf.Bytes(), nil +} + +// createTarballToDisk builds a compressed tar.gz archive and writes it directly to the specified file path. +// Creates the output file at the specified path and writes a gzip-compressed tar archive. +// The metadata.yaml file is always written first at the root of the archive. +// Skips any existing "_templates/metadata.yaml" file to avoid duplication. +// All files are written with 0644 permissions in the archive. +// Properly closes writers to ensure all data is flushed to disk before returning. +func (a *ArtifactBuilder) createTarballToDisk(outputPath string, metadata []byte) error { + outputFile, err := a.shims.Create(outputPath) if err != nil { - return "", fmt.Errorf("failed to create output file: %w", err) + return fmt.Errorf("failed to create output file: %w", err) } defer outputFile.Close() @@ -183,11 +346,11 @@ func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) } if err := tarWriter.WriteHeader(metadataHeader); err != nil { - return "", fmt.Errorf("failed to write metadata header: %w", err) + return fmt.Errorf("failed to write metadata header: %w", err) } if _, err := tarWriter.Write(metadata); err != nil { - return "", fmt.Errorf("failed to write metadata: %w", err) + return fmt.Errorf("failed to write metadata: %w", err) } for path, content := range a.files { @@ -202,25 +365,75 @@ func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) } if err := tarWriter.WriteHeader(header); err != nil { - return "", fmt.Errorf("failed to write header for %s: %w", path, err) + return fmt.Errorf("failed to write header for %s: %w", path, err) } if _, err := tarWriter.Write(content); err != nil { - return "", fmt.Errorf("failed to write content for %s: %w", path, err) + return fmt.Errorf("failed to write content for %s: %w", path, err) } } - return finalOutputPath, nil + return nil } -// ============================================================================= -// Private Methods -// ============================================================================= +// createFluxCDCompatibleImage constructs an OCI image from a layer with FluxCD-specific media types and annotations. +// Creates a FluxCD-compatible config file with empty JSON content as required by FluxCD. +// Sets architecture to amd64 and OS to linux, though FluxCD doesn't enforce these for config artifacts. +// Applies FluxCD-specific media types: manifest v1+json and flux config v1+json. +// Adds comprehensive OCI annotations including creation time, source, revision, title, and version. +// Returns a complete OCI image ready for pushing to FluxCD-compatible registries. +func (a *ArtifactBuilder) createFluxCDCompatibleImage(layer v1.Layer, repoName, tagName string) (v1.Image, error) { + configFile := &v1.ConfigFile{ + Architecture: "amd64", + OS: "linux", + Config: v1.Config{ + Labels: map[string]string{ + "org.opencontainers.image.title": repoName, + "org.opencontainers.image.description": fmt.Sprintf("FluxCD artifact for %s", repoName), + }, + }, + RootFS: v1.RootFS{ + Type: "layers", + }, + History: []v1.History{ + { + Created: v1.Time{Time: time.Now()}, + Comment: "FluxCD artifact layer", + }, + }, + } + + img, err := a.shims.AppendLayers(a.shims.EmptyImage(), layer) + if err != nil { + return nil, fmt.Errorf("failed to append layer to image: %w", err) + } + + img, err = a.shims.ConfigFile(img, configFile) + if err != nil { + return nil, fmt.Errorf("failed to set config file: %w", err) + } + + img = a.shims.MediaType(img, "application/vnd.oci.image.manifest.v1+json") + img = a.shims.ConfigMediaType(img, "application/vnd.cncf.flux.config.v1+json") + + annotations := map[string]string{ + "org.opencontainers.image.created": time.Now().UTC().Format(time.RFC3339), + "org.opencontainers.image.source": "windsor-cli", + "org.opencontainers.image.revision": "latest", + "org.opencontainers.image.title": repoName, + "org.opencontainers.image.version": tagName, + } + + img = a.shims.Annotations(img, annotations) + + return img, nil +} -// resolveOutputPath determines the final output path for the artifact. +// resolveOutputPath determines the final output file path based on the provided path and artifact metadata. // If outputPath is a directory or ends with slash, generates filename from name and version in that directory. // If outputPath appears to be a directory path (no extension), generates filename from name and version. // Otherwise uses outputPath as-is for the filename (user provided explicit filename). +// Generated filenames follow the pattern: {name}-{version}.tar.gz. func (a *ArtifactBuilder) resolveOutputPath(outputPath, name, version string) string { filename := fmt.Sprintf("%s-%s.tar.gz", name, version) @@ -239,65 +452,85 @@ func (a *ArtifactBuilder) resolveOutputPath(outputPath, name, version string) st return outputPath } -// generateMetadataWithNameVersion creates metadata for bundled artifacts with final name and version. -// It combines input metadata with git provenance and builder information, then marshals +// generateMetadataWithNameVersion creates complete blueprint metadata by merging input metadata with name and version. +// Combines input metadata with git provenance and builder information, then marshals // the complete metadata structure to YAML for embedding in the artifact. +// Git provenance and builder info are best-effort - failures result in empty values rather than errors. +// Includes timestamp in RFC3339 format for artifact creation tracking. +// Returns marshaled YAML bytes ready for inclusion in tar archives. func (a *ArtifactBuilder) generateMetadataWithNameVersion(input BlueprintMetadataInput, name, version string) ([]byte, error) { - gitInfo, _ := a.getGitProvenance() - builderInfo, _ := a.getBuilderInfo() + gitProvenance, err := a.getGitProvenance() + if err != nil { + gitProvenance = GitProvenance{} + } + + builderInfo, err := a.getBuilderInfo() + if err != nil { + builderInfo = BuilderInfo{} + } metadata := BlueprintMetadata{ Name: name, - Version: version, Description: input.Description, + Version: version, Author: input.Author, Tags: input.Tags, Homepage: input.Homepage, License: input.License, Timestamp: time.Now().UTC().Format(time.RFC3339), - Git: gitInfo, + Git: gitProvenance, Builder: builderInfo, } return a.shims.YamlMarshal(metadata) } -// getGitProvenance extracts git repository information for provenance tracking. -// All git operations are best-effort and failures are ignored since git provenance is optional. -// Returns empty values for any git operations that fail. +// getGitProvenance retrieves git repository information including commit SHA, tag, and remote URL. +// Extracts git repository information for provenance tracking using shell commands. +// Requires shell dependency to be available for git command execution. +// Gets current commit SHA, attempts to find exact tag match for HEAD, and retrieves origin URL. +// Tag lookup uses exact match only - returns empty string if HEAD is not tagged. +// All git operations return errors if commands fail, unlike the best-effort approach in some functions. func (a *ArtifactBuilder) getGitProvenance() (GitProvenance, error) { - var gitInfo GitProvenance - - if commitSHA, err := a.shell.ExecSilent("git", "rev-parse", "HEAD"); err == nil { - gitInfo.CommitSHA = strings.TrimSpace(commitSHA) + commitSHA, err := a.shell.ExecSilent("git", "rev-parse", "HEAD") + if err != nil { + return GitProvenance{}, fmt.Errorf("failed to get commit SHA: %w", err) } - if tag, err := a.shell.ExecSilent("git", "tag", "--points-at", "HEAD"); err == nil { - gitInfo.Tag = strings.TrimSpace(tag) - } + tag, _ := a.shell.ExecSilent("git", "describe", "--tags", "--exact-match", "HEAD") - if remoteURL, err := a.shell.ExecSilent("git", "config", "--get", "remote.origin.url"); err == nil { - gitInfo.RemoteURL = strings.TrimSpace(remoteURL) + remoteURL, err := a.shell.ExecSilent("git", "config", "--get", "remote.origin.url") + if err != nil { + remoteURL = "" } - return gitInfo, nil + return GitProvenance{ + CommitSHA: strings.TrimSpace(commitSHA), + Tag: strings.TrimSpace(tag), + RemoteURL: strings.TrimSpace(remoteURL), + }, nil } -// getBuilderInfo extracts information about who/what built the artifact. -// It retrieves git user name and email configuration. All git operations are -// best-effort and failures are ignored since builder info is optional. +// getBuilderInfo retrieves information about the current user building the artifact. +// Extracts information about who/what built the artifact using git configuration. +// Retrieves git user name and email configuration from git global or repository config. +// Returns empty strings for missing configuration rather than errors for optional builder info. +// Used for audit trails and artifact attribution in generated metadata. func (a *ArtifactBuilder) getBuilderInfo() (BuilderInfo, error) { - var builderInfo BuilderInfo - - if user, err := a.shell.ExecSilent("git", "config", "user.name"); err == nil { - builderInfo.User = strings.TrimSpace(user) + user, err := a.shell.ExecSilent("git", "config", "--get", "user.name") + if err != nil { + user = "" } - if email, err := a.shell.ExecSilent("git", "config", "user.email"); err == nil { - builderInfo.Email = strings.TrimSpace(email) + email, err := a.shell.ExecSilent("git", "config", "--get", "user.email") + if err != nil { + email = "" } - return builderInfo, nil + return BuilderInfo{ + User: strings.TrimSpace(user), + Email: strings.TrimSpace(email), + }, nil } // Ensure ArtifactBuilder implements Artifact interface diff --git a/pkg/bundler/artifact_test.go b/pkg/bundler/artifact_test.go index 0041f3c42..33a1a8a78 100644 --- a/pkg/bundler/artifact_test.go +++ b/pkg/bundler/artifact_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/shell" ) @@ -468,7 +470,7 @@ description: A test project if err == nil { t.Error("Expected error when name is missing") } - if !strings.Contains(err.Error(), "name is required") { + if !strings.Contains(err.Error(), "name is required: provide via tag parameter or metadata.yaml") { t.Errorf("Expected name required error, got: %v", err) } }) @@ -493,7 +495,7 @@ description: A test project if err == nil { t.Error("Expected error when version is missing") } - if !strings.Contains(err.Error(), "version is required") { + if !strings.Contains(err.Error(), "version is required: provide via tag parameter or metadata.yaml") { t.Errorf("Expected version required error, got: %v", err) } }) @@ -748,6 +750,274 @@ description: A test project }) } +func TestArtifactBuilder_Push(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("ErrorWithInvalidTagFormat", func(t *testing.T) { + // Given a builder + builder, mocks := setup(t) + + // Create an artifact first + mocks.Shims.Create = func(name string) (io.WriteCloser, error) { + return os.Create(name) + } + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + _, err := builder.Create("test.tar.gz", "") + if err != nil { + t.Fatalf("Failed to create artifact: %v", err) + } + + // When pushing with invalid tag format + invalidTags := []string{ + "notag", + "only:colon:", + ":missingname", + "missingversion:", + } + + for _, tag := range invalidTags { + err := builder.Push("registry.example.com", tag) + if err == nil || !strings.Contains(err.Error(), "tag must be in format") { + t.Errorf("Expected tag format error for %s, got: %v", tag, err) + } + } + }) + + t.Run("ErrorWhenNameMissing", func(t *testing.T) { + // Given a builder with no metadata and no tag provided + builder, mocks := setup(t) + + // Set up mocks for metadata parsing that returns empty metadata + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + // Return empty metadata (no name or version) + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("")) + + // When pushing without providing tag + err := builder.Push("registry.example.com", "") + if err == nil || !strings.Contains(err.Error(), "name is required") { + t.Errorf("Expected name required error, got: %v", err) + } + }) + + t.Run("ErrorWhenVersionMissing", func(t *testing.T) { + // Given a builder with incomplete metadata (name but no version) + builder, mocks := setup(t) + + // Set up mocks for metadata parsing that returns only name + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + // No version set + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("name: test")) + + // When pushing without providing tag + err := builder.Push("registry.example.com", "") + if err == nil || !strings.Contains(err.Error(), "version is required") { + t.Errorf("Expected version required error, got: %v", err) + } + }) + + t.Run("SuccessWithValidMetadata", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up mocks for successful in-memory operation + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("mock implementation error") + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "myapp:2.0.0") + + // Then should get mock implementation error (indicating it reached the OCI creation step) + if err == nil || !strings.Contains(err.Error(), "mock implementation error") { + t.Errorf("Expected mock implementation error, got: %v", err) + } + }) + + t.Run("SuccessWithInMemoryOperation", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up mocks for successful in-memory operation + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { + return []byte("test: yaml"), nil + } + + // Mock the tarball creation to verify in-memory operation + tarballCreated := false + mocks.Shims.NewGzipWriter = func(w io.Writer) *gzip.Writer { + tarballCreated = true + return gzip.NewWriter(w) + } + + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + // Verify that we got here with in-memory content + if !tarballCreated { + return nil, fmt.Errorf("tarball was not created in memory") + } + return nil, fmt.Errorf("expected test termination") + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + builder.AddFile("test.txt", []byte("test content")) + + // When pushing (this should work entirely in-memory) + err := builder.Push("registry.example.com", "test:1.0.0") + + // Then should get expected test termination (proving in-memory operation worked) + if err == nil || !strings.Contains(err.Error(), "expected test termination") { + t.Errorf("Expected test termination error, got: %v", err) + } + + // And tarball should have been created in memory + if !tarballCreated { + t.Error("Expected tarball to be created in memory") + } + }) + + t.Run("ErrorWithInvalidRepositoryReference", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing with invalid registry format (contains invalid characters) + err := builder.Push("invalid registry format with spaces", "test:1.0.0") + + // Then should get repository reference error + if err == nil || !strings.Contains(err.Error(), "invalid repository reference") { + t.Errorf("Expected invalid repository reference error, got: %v", err) + } + }) + + t.Run("ErrorFromCreateTarballInMemory", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + + // Mock tar writer to fail on WriteHeader + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeHeaderFunc: func(*tar.Header) error { + return fmt.Errorf("tar writer header failed") + }, + } + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "test:1.0.0") + + // Then should get tarball creation error + if err == nil || !strings.Contains(err.Error(), "failed to create tarball in memory") { + t.Errorf("Expected tarball creation error, got: %v", err) + } + }) + + t.Run("ErrorFromCreateFluxCDCompatibleImage", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { + return []byte("test: yaml"), nil + } + + // Mock AppendLayers to fail + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("config file mutation failed") + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "test:1.0.0") + + // Then should get FluxCD image creation error + if err == nil || !strings.Contains(err.Error(), "failed to create FluxCD-compatible OCI image") { + t.Errorf("Expected FluxCD image creation error, got: %v", err) + } + }) + + t.Run("SuccessWithEmptyTag", func(t *testing.T) { + // Given a builder with valid metadata file + builder, mocks := setup(t) + + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "myapp" + input.Version = "2.0.0" + return nil + } + mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { + return []byte("test: yaml"), nil + } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("expected test termination") + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: myapp\nversion: 2.0.0")) + + // When pushing with empty tag (should use metadata values) + err := builder.Push("registry.example.com", "") + + // Then should use metadata name/version and reach the expected termination + if err == nil || !strings.Contains(err.Error(), "expected test termination") { + t.Errorf("Expected test termination error, got: %v", err) + } + }) +} + // ============================================================================= // Test Private Methods // ============================================================================= @@ -831,3 +1101,573 @@ func TestArtifactBuilder_resolveOutputPath(t *testing.T) { } }) } + +func TestArtifactBuilder_createTarballInMemory(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("ErrorWhenTarWriterWriteHeaderFails", func(t *testing.T) { + // Given a builder with files + builder, mocks := setup(t) + builder.AddFile("test.txt", []byte("content")) + + // Mock tar writer to fail on WriteHeader + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeHeaderFunc: func(*tar.Header) error { + return fmt.Errorf("write header failed") + }, + } + } + + // When creating tarball in memory + _, err := builder.createTarballInMemory([]byte("metadata")) + + // Then should get write header error + if err == nil || !strings.Contains(err.Error(), "failed to write metadata header") { + t.Errorf("Expected write header error, got: %v", err) + } + }) + + t.Run("ErrorWhenTarWriterWriteFails", func(t *testing.T) { + // Given a builder with files + builder, mocks := setup(t) + builder.AddFile("test.txt", []byte("content")) + + // Mock tar writer to fail on Write + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeFunc: func([]byte) (int, error) { + return 0, fmt.Errorf("write failed") + }, + } + } + + // When creating tarball in memory + _, err := builder.createTarballInMemory([]byte("metadata")) + + // Then should get write error + if err == nil || !strings.Contains(err.Error(), "failed to write metadata") { + t.Errorf("Expected write error, got: %v", err) + } + }) + + t.Run("ErrorWhenFileHeaderWriteFails", func(t *testing.T) { + // Given a builder with files + builder, mocks := setup(t) + builder.AddFile("test.txt", []byte("content")) + + headerCount := 0 + // Mock tar writer to fail on second WriteHeader (for file) + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeHeaderFunc: func(*tar.Header) error { + headerCount++ + if headerCount > 1 { + return fmt.Errorf("file header write failed") + } + return nil + }, + writeFunc: func([]byte) (int, error) { + return 100, nil // Success for metadata write + }, + } + } + + // When creating tarball in memory + _, err := builder.createTarballInMemory([]byte("metadata")) + + // Then should get file header error + if err == nil || !strings.Contains(err.Error(), "failed to write header for test.txt") { + t.Errorf("Expected file header error, got: %v", err) + } + }) + + t.Run("ErrorWhenFileContentWriteFails", func(t *testing.T) { + // Given a builder with files + builder, mocks := setup(t) + builder.AddFile("test.txt", []byte("content")) + + writeCount := 0 + // Mock tar writer to fail on second Write (for file content) + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeFunc: func([]byte) (int, error) { + writeCount++ + if writeCount > 1 { + return 0, fmt.Errorf("file content write failed") + } + return 100, nil // Success for metadata write + }, + } + } + + // When creating tarball in memory + _, err := builder.createTarballInMemory([]byte("metadata")) + + // Then should get file content error + if err == nil || !strings.Contains(err.Error(), "failed to write content for test.txt") { + t.Errorf("Expected file content error, got: %v", err) + } + }) +} + +// ============================================================================= +// Test generateMetadataWithNameVersion +// ============================================================================= + +func TestArtifactBuilder_generateMetadataWithNameVersion(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("SuccessWithGitProvenanceAndBuilderInfo", func(t *testing.T) { + // Given a builder with shell configured + builder, mocks := setup(t) + + // Mock successful git operations + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git rev-parse HEAD"): + return "abc123def456", nil + case strings.Contains(cmd, "git describe --tags --exact-match HEAD"): + return "v1.0.0", nil + case strings.Contains(cmd, "git config --get remote.origin.url"): + return "https://github.com/example/repo.git", nil + case strings.Contains(cmd, "git config --get user.name"): + return "Test User", nil + case strings.Contains(cmd, "git config --get user.email"): + return "test@example.com", nil + default: + return "", nil + } + } + + input := BlueprintMetadataInput{ + Description: "Test description", + Author: "Test Author", + Tags: []string{"test", "example"}, + Homepage: "https://example.com", + License: "MIT", + } + + // When generating metadata + metadata, err := builder.generateMetadataWithNameVersion(input, "testapp", "1.0.0") + + // Then should succeed + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if metadata == nil { + t.Error("Expected metadata to be generated") + } + }) + + t.Run("SuccessWithGitProvenanceFailure", func(t *testing.T) { + // Given a builder with shell configured to fail git operations + builder, mocks := setup(t) + + // Mock git operations to fail + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + return "", fmt.Errorf("git command failed") + } + + input := BlueprintMetadataInput{ + Description: "Test description", + } + + // When generating metadata + metadata, err := builder.generateMetadataWithNameVersion(input, "testapp", "1.0.0") + + // Then should succeed with empty git provenance + if err != nil { + t.Errorf("Expected success despite git failures, got error: %v", err) + } + if metadata == nil { + t.Error("Expected metadata to be generated") + } + }) + + t.Run("ErrorWhenYamlMarshalFails", func(t *testing.T) { + // Given a builder with failing YAML marshal + builder, mocks := setup(t) + mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { + return nil, fmt.Errorf("yaml marshal failed") + } + + input := BlueprintMetadataInput{} + + // When generating metadata + _, err := builder.generateMetadataWithNameVersion(input, "testapp", "1.0.0") + + // Then should get marshal error + if err == nil || !strings.Contains(err.Error(), "yaml marshal failed") { + t.Errorf("Expected yaml marshal error, got: %v", err) + } + }) +} + +func TestArtifactBuilder_getGitProvenance(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("SuccessWithAllGitInfo", func(t *testing.T) { + // Given a builder with successful git operations + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git rev-parse HEAD"): + return " abc123def456 ", nil // With whitespace to test trimming + case strings.Contains(cmd, "git describe --tags --exact-match HEAD"): + return " v1.0.0 ", nil // With whitespace to test trimming + case strings.Contains(cmd, "git config --get remote.origin.url"): + return " https://github.com/example/repo.git ", nil // With whitespace + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting git provenance + provenance, err := builder.getGitProvenance() + + // Then should succeed with trimmed values + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if provenance.CommitSHA != "abc123def456" { + t.Errorf("Expected commit SHA 'abc123def456', got '%s'", provenance.CommitSHA) + } + if provenance.Tag != "v1.0.0" { + t.Errorf("Expected tag 'v1.0.0', got '%s'", provenance.Tag) + } + if provenance.RemoteURL != "https://github.com/example/repo.git" { + t.Errorf("Expected remote URL 'https://github.com/example/repo.git', got '%s'", provenance.RemoteURL) + } + }) + + t.Run("ErrorWhenCommitSHAFails", func(t *testing.T) { + // Given a builder with failing commit SHA command + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + if strings.Contains(cmd, "git rev-parse HEAD") { + return "", fmt.Errorf("not a git repository") + } + return "", nil + } + + // When getting git provenance + _, err := builder.getGitProvenance() + + // Then should get commit SHA error + if err == nil || !strings.Contains(err.Error(), "failed to get commit SHA") { + t.Errorf("Expected commit SHA error, got: %v", err) + } + }) + + t.Run("SuccessWithMissingTag", func(t *testing.T) { + // Given a builder where tag command fails but others succeed + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git rev-parse HEAD"): + return "abc123def456", nil + case strings.Contains(cmd, "git describe --tags --exact-match HEAD"): + return "", fmt.Errorf("no tag found") // Tag command fails + case strings.Contains(cmd, "git config --get remote.origin.url"): + return "https://github.com/example/repo.git", nil + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting git provenance + provenance, err := builder.getGitProvenance() + + // Then should succeed with empty tag + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if provenance.CommitSHA != "abc123def456" { + t.Errorf("Expected commit SHA 'abc123def456', got '%s'", provenance.CommitSHA) + } + if provenance.Tag != "" { + t.Errorf("Expected empty tag, got '%s'", provenance.Tag) + } + if provenance.RemoteURL != "https://github.com/example/repo.git" { + t.Errorf("Expected remote URL, got '%s'", provenance.RemoteURL) + } + }) + + t.Run("SuccessWithMissingRemoteURL", func(t *testing.T) { + // Given a builder where remote URL command fails + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git rev-parse HEAD"): + return "abc123def456", nil + case strings.Contains(cmd, "git describe --tags --exact-match HEAD"): + return "v1.0.0", nil + case strings.Contains(cmd, "git config --get remote.origin.url"): + return "", fmt.Errorf("no remote configured") // Remote URL fails + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting git provenance + provenance, err := builder.getGitProvenance() + + // Then should succeed with empty remote URL + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if provenance.CommitSHA != "abc123def456" { + t.Errorf("Expected commit SHA 'abc123def456', got '%s'", provenance.CommitSHA) + } + if provenance.Tag != "v1.0.0" { + t.Errorf("Expected tag 'v1.0.0', got '%s'", provenance.Tag) + } + if provenance.RemoteURL != "" { + t.Errorf("Expected empty remote URL, got '%s'", provenance.RemoteURL) + } + }) +} + +func TestArtifactBuilder_getBuilderInfo(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("SuccessWithUserAndEmail", func(t *testing.T) { + // Given a builder with configured git user info + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git config --get user.name"): + return " Test User ", nil // With whitespace to test trimming + case strings.Contains(cmd, "git config --get user.email"): + return " test@example.com ", nil // With whitespace to test trimming + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting builder info + builderInfo, err := builder.getBuilderInfo() + + // Then should succeed with trimmed values + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if builderInfo.User != "Test User" { + t.Errorf("Expected user 'Test User', got '%s'", builderInfo.User) + } + if builderInfo.Email != "test@example.com" { + t.Errorf("Expected email 'test@example.com', got '%s'", builderInfo.Email) + } + }) + + t.Run("SuccessWithMissingUserName", func(t *testing.T) { + // Given a builder where user name is not configured + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git config --get user.name"): + return "", fmt.Errorf("user.name not configured") + case strings.Contains(cmd, "git config --get user.email"): + return "test@example.com", nil + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting builder info + builderInfo, err := builder.getBuilderInfo() + + // Then should succeed with empty user name + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if builderInfo.User != "" { + t.Errorf("Expected empty user, got '%s'", builderInfo.User) + } + if builderInfo.Email != "test@example.com" { + t.Errorf("Expected email 'test@example.com', got '%s'", builderInfo.Email) + } + }) + + t.Run("SuccessWithMissingEmail", func(t *testing.T) { + // Given a builder where email is not configured + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + cmd := strings.Join(append([]string{command}, args...), " ") + switch { + case strings.Contains(cmd, "git config --get user.name"): + return "Test User", nil + case strings.Contains(cmd, "git config --get user.email"): + return "", fmt.Errorf("user.email not configured") + default: + return "", fmt.Errorf("unexpected command: %s", cmd) + } + } + + // When getting builder info + builderInfo, err := builder.getBuilderInfo() + + // Then should succeed with empty email + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if builderInfo.User != "Test User" { + t.Errorf("Expected user 'Test User', got '%s'", builderInfo.User) + } + if builderInfo.Email != "" { + t.Errorf("Expected empty email, got '%s'", builderInfo.Email) + } + }) + + t.Run("SuccessWithBothMissing", func(t *testing.T) { + // Given a builder where both user and email are not configured + builder, mocks := setup(t) + + mocks.Shell.ExecSilentFunc = func(command string, args ...string) (string, error) { + return "", fmt.Errorf("git config not found") + } + + // When getting builder info + builderInfo, err := builder.getBuilderInfo() + + // Then should succeed with empty values + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if builderInfo.User != "" { + t.Errorf("Expected empty user, got '%s'", builderInfo.User) + } + if builderInfo.Email != "" { + t.Errorf("Expected empty email, got '%s'", builderInfo.Email) + } + }) +} + +func TestArtifactBuilder_createFluxCDCompatibleImage(t *testing.T) { + setup := func(t *testing.T) (*ArtifactBuilder, *ArtifactMocks) { + t.Helper() + mocks := setupArtifactMocks(t) + builder := NewArtifactBuilder() + builder.shims = mocks.Shims + builder.Initialize(mocks.Injector) + return builder, mocks + } + + t.Run("ErrorWhenAppendLayersFails", func(t *testing.T) { + // Given a builder with failing AppendLayers + builder, mocks := setup(t) + + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("append layers failed") + } + + // When creating FluxCD compatible image + _, err := builder.createFluxCDCompatibleImage(nil, "test-repo", "v1.0.0") + + // Then should get append layers error + if err == nil || !strings.Contains(err.Error(), "failed to append layer to image") { + t.Errorf("Expected append layer error, got: %v", err) + } + }) + + t.Run("SuccessWithValidLayer", func(t *testing.T) { + // Given a builder with successful shim operations + builder, mocks := setup(t) + + // Mock successful image creation + mockImage := &mockImage{} + mocks.Shims.EmptyImage = func() v1.Image { return mockImage } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return mockImage, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + // Verify config file has expected properties + if cfg.Architecture != "amd64" { + return nil, fmt.Errorf("expected amd64 architecture, got %s", cfg.Architecture) + } + if cfg.OS != "linux" { + return nil, fmt.Errorf("expected linux OS, got %s", cfg.OS) + } + if cfg.Config.Labels["org.opencontainers.image.title"] != "test-repo" { + return nil, fmt.Errorf("expected title label to be test-repo") + } + return mockImage, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { return mockImage } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { return mockImage } + mocks.Shims.Annotations = func(img v1.Image, anns map[string]string) v1.Image { return mockImage } + + // When creating FluxCD compatible image + img, err := builder.createFluxCDCompatibleImage(nil, "test-repo", "v1.0.0") + + // Then should succeed + if err != nil { + t.Errorf("Expected success, got error: %v", err) + } + if img == nil { + t.Error("Expected to receive a non-nil image") + } + }) +} + +// ============================================================================= +// Test Helpers +// ============================================================================= + +// mockImage provides a mock implementation of v1.Image for testing +type mockImage struct{} + +func (m *mockImage) Layers() ([]v1.Layer, error) { return nil, nil } +func (m *mockImage) MediaType() (types.MediaType, error) { return "", nil } +func (m *mockImage) Size() (int64, error) { return 0, nil } +func (m *mockImage) ConfigName() (v1.Hash, error) { return v1.Hash{}, nil } +func (m *mockImage) ConfigFile() (*v1.ConfigFile, error) { return nil, nil } +func (m *mockImage) RawConfigFile() ([]byte, error) { return nil, nil } +func (m *mockImage) Digest() (v1.Hash, error) { return v1.Hash{}, nil } +func (m *mockImage) Manifest() (*v1.Manifest, error) { return nil, nil } +func (m *mockImage) RawManifest() ([]byte, error) { return nil, nil } +func (m *mockImage) LayerByDigest(v1.Hash) (v1.Layer, error) { return nil, nil } +func (m *mockImage) LayerByDiffID(v1.Hash) (v1.Layer, error) { return nil, nil } diff --git a/pkg/bundler/mock_artifact.go b/pkg/bundler/mock_artifact.go index 71761246f..48b87f27e 100644 --- a/pkg/bundler/mock_artifact.go +++ b/pkg/bundler/mock_artifact.go @@ -18,6 +18,7 @@ type MockArtifact struct { InitializeFunc func(injector di.Injector) error AddFileFunc func(path string, content []byte) error CreateFunc func(outputPath string, tag string) (string, error) + PushFunc func(registry string, tag string) error } // ============================================================================= @@ -57,5 +58,13 @@ func (m *MockArtifact) Create(outputPath string, tag string) (string, error) { return "", nil } +// Push calls the mock PushFunc if set, otherwise returns nil +func (m *MockArtifact) Push(registry string, tag string) error { + if m.PushFunc != nil { + return m.PushFunc(registry, tag) + } + return nil +} + // Ensure MockArtifact implements Artifact interface var _ Artifact = (*MockArtifact)(nil) diff --git a/pkg/bundler/mock_artifact_test.go b/pkg/bundler/mock_artifact_test.go index 1e922049f..d081b814d 100644 --- a/pkg/bundler/mock_artifact_test.go +++ b/pkg/bundler/mock_artifact_test.go @@ -137,3 +137,48 @@ func TestMockArtifact_Create(t *testing.T) { } }) } + +func TestMockArtifact_Push(t *testing.T) { + t.Run("Success", func(t *testing.T) { + // Given a mock with a custom push function + mock := NewMockArtifact() + called := false + var capturedRegistry, capturedTag string + mock.PushFunc = func(registry string, tag string) error { + called = true + capturedRegistry = registry + capturedTag = tag + return nil + } + + // When calling Push + err := mock.Push("registry.example.com", "myapp:v1.0.0") + + // Then the mock function should be called + if !called { + t.Error("Expected PushFunc to be called") + } + if capturedRegistry != "registry.example.com" { + t.Errorf("Expected registry 'registry.example.com', got '%s'", capturedRegistry) + } + if capturedTag != "myapp:v1.0.0" { + t.Errorf("Expected tag 'myapp:v1.0.0', got '%s'", capturedTag) + } + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("NotImplemented", func(t *testing.T) { + // Given a mock with no custom push function + mock := NewMockArtifact() + + // When calling Push + err := mock.Push("registry.example.com", "myapp:v1.0.0") + + // Then no error should be returned + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) +} diff --git a/pkg/bundler/shims.go b/pkg/bundler/shims.go index 3354d5e09..a0c8012e7 100644 --- a/pkg/bundler/shims.go +++ b/pkg/bundler/shims.go @@ -8,6 +8,10 @@ import ( "path/filepath" "github.com/goccy/go-yaml" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/types" ) // The Shims provides mockable wrappers around system and file operations for testing. @@ -28,15 +32,21 @@ type TarWriter interface { // Shims provides mockable wrappers around system and file operations type Shims struct { - Stat func(name string) (os.FileInfo, error) - Create func(name string) (io.WriteCloser, error) - ReadFile func(name string) ([]byte, error) - Walk func(root string, walkFn filepath.WalkFunc) error - NewGzipWriter func(w io.Writer) *gzip.Writer - NewTarWriter func(w io.Writer) TarWriter - YamlUnmarshal func(data []byte, v any) error - FilepathRel func(basepath, targpath string) (string, error) - YamlMarshal func(data any) ([]byte, error) + Stat func(name string) (os.FileInfo, error) + Create func(name string) (io.WriteCloser, error) + ReadFile func(name string) ([]byte, error) + Walk func(root string, walkFn filepath.WalkFunc) error + NewGzipWriter func(w io.Writer) *gzip.Writer + NewTarWriter func(w io.Writer) TarWriter + YamlUnmarshal func(data []byte, v any) error + FilepathRel func(basepath, targpath string) (string, error) + YamlMarshal func(data any) ([]byte, error) + AppendLayers func(base v1.Image, layers ...v1.Layer) (v1.Image, error) + ConfigFile func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) + MediaType func(img v1.Image, mt types.MediaType) v1.Image + ConfigMediaType func(img v1.Image, mt types.MediaType) v1.Image + Annotations func(img v1.Image, anns map[string]string) v1.Image + EmptyImage func() v1.Image } // ============================================================================= @@ -48,13 +58,21 @@ func NewShims() *Shims { return &Shims{ Stat: os.Stat, // #nosec G304 - User-controlled output path is intentional for build artifact creation - Create: func(name string) (io.WriteCloser, error) { return os.Create(name) }, - ReadFile: os.ReadFile, - Walk: filepath.Walk, - NewGzipWriter: gzip.NewWriter, - NewTarWriter: func(w io.Writer) TarWriter { return tar.NewWriter(w) }, - YamlUnmarshal: yaml.Unmarshal, - FilepathRel: filepath.Rel, - YamlMarshal: yaml.Marshal, + Create: func(name string) (io.WriteCloser, error) { return os.Create(name) }, + ReadFile: os.ReadFile, + Walk: filepath.Walk, + NewGzipWriter: gzip.NewWriter, + NewTarWriter: func(w io.Writer) TarWriter { return tar.NewWriter(w) }, + YamlUnmarshal: yaml.Unmarshal, + FilepathRel: filepath.Rel, + YamlMarshal: yaml.Marshal, + AppendLayers: mutate.AppendLayers, + ConfigFile: mutate.ConfigFile, + MediaType: mutate.MediaType, + ConfigMediaType: mutate.ConfigMediaType, + Annotations: func(img v1.Image, anns map[string]string) v1.Image { + return mutate.Annotations(img, anns).(v1.Image) + }, + EmptyImage: func() v1.Image { return empty.Image }, } } diff --git a/pkg/controller/mock_controller.go b/pkg/controller/mock_controller.go index a8d717406..c40b3b03f 100644 --- a/pkg/controller/mock_controller.go +++ b/pkg/controller/mock_controller.go @@ -5,6 +5,7 @@ import ( secretsConfigType "github.com/windsorcli/cli/api/v1alpha1/secrets" "github.com/windsorcli/cli/pkg/blueprint" + "github.com/windsorcli/cli/pkg/bundler" "github.com/windsorcli/cli/pkg/cluster" "github.com/windsorcli/cli/pkg/config" "github.com/windsorcli/cli/pkg/di" @@ -47,6 +48,8 @@ type MockController struct { ResolveKubernetesManagerFunc func() kubernetes.KubernetesManager ResolveKubernetesClientFunc func() kubernetes.KubernetesClient ResolveClusterClientFunc func() cluster.ClusterClient + ResolveArtifactBuilderFunc func() bundler.Artifact + ResolveAllBundlersFunc func() []bundler.Bundler WriteConfigurationFilesFunc func() error SetEnvironmentVariablesFunc func() error } @@ -202,6 +205,17 @@ func NewMockConstructors() ComponentConstructors { NewTalosClusterClient: func(injector di.Injector) *cluster.TalosClusterClient { return cluster.NewTalosClusterClient(injector) }, + + // Bundler components + NewArtifactBuilder: func(injector di.Injector) bundler.Artifact { + return bundler.NewMockArtifact() + }, + NewTemplateBundler: func(injector di.Injector) bundler.Bundler { + return bundler.NewMockBundler() + }, + NewKustomizeBundler: func(injector di.Injector) bundler.Bundler { + return bundler.NewMockBundler() + }, } } @@ -400,5 +414,21 @@ func (m *MockController) ResolveClusterClient() cluster.ClusterClient { return m.BaseController.ResolveClusterClient() } +// ResolveArtifactBuilder implements the Controller interface +func (m *MockController) ResolveArtifactBuilder() bundler.Artifact { + if m.ResolveArtifactBuilderFunc != nil { + return m.ResolveArtifactBuilderFunc() + } + return m.BaseController.ResolveArtifactBuilder() +} + +// ResolveAllBundlers implements the Controller interface +func (m *MockController) ResolveAllBundlers() []bundler.Bundler { + if m.ResolveAllBundlersFunc != nil { + return m.ResolveAllBundlersFunc() + } + return m.BaseController.ResolveAllBundlers() +} + // Ensure MockController implements Controller var _ Controller = (*MockController)(nil) From 098ebbb9210cde773fed3443573d13cef2103697 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Wed, 2 Jul 2025 11:47:45 -0400 Subject: [PATCH 2/3] feat(cmd): Add windsor push for pushing OCI artifacts --- cmd/push.go | 24 +- cmd/push_test.go | 121 +++++- pkg/bundler/artifact.go | 141 ++++-- pkg/bundler/artifact_test.go | 698 ++++++++++++++++++++++++++---- pkg/bundler/mock_artifact.go | 6 +- pkg/bundler/mock_artifact_test.go | 22 +- pkg/bundler/shims.go | 40 +- 7 files changed, 880 insertions(+), 172 deletions(-) diff --git a/cmd/push.go b/cmd/push.go index 0ae10e49b..c8b10cd5e 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -16,7 +16,7 @@ var pushCmd = &cobra.Command{ This command packages your blueprint and pushes it to any OCI-compatible registry like Docker Hub, GitHub Container Registry, or AWS ECR. The artifacts are compatible -with FluxCD and other GitOps tools. +with FluxCD's OCIRepository. Examples: # Push to Docker Hub @@ -52,32 +52,38 @@ Examples: } } - // Parse registry and tag from positional argument + // Parse registry, repository name, and tag from positional argument if len(args) == 0 { return fmt.Errorf("registry is required: windsor push registry/repo[:tag]") } - var registry, tag string + var registryBase, repoName, tag string arg := args[0] + // First extract tag if present if lastColon := strings.LastIndex(arg, ":"); lastColon > 0 && lastColon < len(arg)-1 { // Has tag in URL format (registry/repo:tag) - registry = arg[:lastColon] tag = arg[lastColon+1:] + arg = arg[:lastColon] // Remove tag from argument + } + + // Now extract repository name (last path component) and registry base + if lastSlash := strings.LastIndex(arg, "/"); lastSlash >= 0 { + registryBase = arg[:lastSlash] + repoName = arg[lastSlash+1:] } else { - // No tag in URL, registry only - registry = arg + return fmt.Errorf("invalid registry format: must include repository path (e.g., registry.com/namespace/repo)") } // Push the artifact to the registry - if err := artifact.Push(registry, tag); err != nil { + if err := artifact.Push(registryBase, repoName, tag); err != nil { return fmt.Errorf("failed to push artifact: %w", err) } if tag != "" { - fmt.Printf("Blueprint pushed successfully to %s:%s\n", registry, tag) + fmt.Printf("Blueprint pushed successfully to %s/%s:%s\n", registryBase, repoName, tag) } else { - fmt.Printf("Blueprint pushed successfully to %s\n", registry) + fmt.Printf("Blueprint pushed successfully to %s/%s\n", registryBase, repoName) } return nil }, diff --git a/cmd/push_test.go b/cmd/push_test.go index 23fe66d19..9af2b2fd9 100644 --- a/cmd/push_test.go +++ b/cmd/push_test.go @@ -41,7 +41,7 @@ contexts: artifactBuilder := bundler.NewMockArtifact() artifactBuilder.InitializeFunc = func(injector di.Injector) error { return nil } artifactBuilder.AddFileFunc = func(path string, content []byte) error { return nil } - artifactBuilder.PushFunc = func(registry string, tag string) error { return nil } + artifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { return nil } // Create mock template bundler templateBundler := bundler.NewMockBundler() @@ -128,6 +128,24 @@ func TestPushCmd(t *testing.T) { } }) + t.Run("ErrorInvalidRegistryFormat", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + + // When executing the push command with invalid registry format (no repository path) + rootCmd.SetArgs([]string{"push", "registry.example.com"}) + err := Execute(mocks.Controller) + + // Then an error should be returned + if err == nil { + t.Error("Expected error, got nil") + } + expectedError := "invalid registry format: must include repository path (e.g., registry.com/namespace/repo)" + if err.Error() != expectedError { + t.Errorf("Expected error %q, got %q", expectedError, err.Error()) + } + }) + t.Run("ErrorInitializingController", func(t *testing.T) { // Given a push environment with failing controller initialization mocks := setupPushMocks(t) @@ -214,7 +232,7 @@ func TestPushCmd(t *testing.T) { t.Run("ErrorArtifactPushFails", func(t *testing.T) { // Given a push environment with failing artifact push mocks := setupPushMocks(t) - mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { return fmt.Errorf("push to registry failed") } @@ -299,8 +317,8 @@ func TestPushCmd(t *testing.T) { mocks := setupPushMocks(t) var receivedRegistry, receivedTag string - mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { - receivedRegistry = registry + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { + receivedRegistry = fmt.Sprintf("%s/%s", registryBase, repoName) receivedTag = tag return nil } @@ -328,7 +346,7 @@ func TestPushCmd(t *testing.T) { mocks := setupPushMocks(t) var receivedTag string - mocks.ArtifactBuilder.PushFunc = func(registry string, tag string) error { + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { receivedTag = tag return nil } @@ -364,4 +382,97 @@ func TestPushCmd(t *testing.T) { t.Errorf("Expected no error, got %v", err) } }) + + t.Run("EdgeCaseColonAtBeginning", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + var receivedRegistry, receivedTag string + + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { + receivedRegistry = fmt.Sprintf("%s/%s", registryBase, repoName) + receivedTag = tag + return nil + } + + // When executing with colon at beginning (should not extract tag) + rootCmd.SetArgs([]string{"push", ":registry.example.com/repo"}) + err := Execute(mocks.Controller) + + // Then no error should be returned (colon at beginning is not treated as tag separator) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And no tag should be extracted + if receivedTag != "" { + t.Errorf("Expected empty tag, got %s", receivedTag) + } + + // And registry should include the colon + if receivedRegistry != ":registry.example.com/repo" { + t.Errorf("Expected registry ':registry.example.com/repo', got %s", receivedRegistry) + } + }) + + t.Run("EdgeCaseColonAtEnd", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + var receivedRegistry, receivedTag string + + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { + receivedRegistry = fmt.Sprintf("%s/%s", registryBase, repoName) + receivedTag = tag + return nil + } + + // When executing with colon at end (should not extract tag) + rootCmd.SetArgs([]string{"push", "registry.example.com/repo:"}) + err := Execute(mocks.Controller) + + // Then no error should be returned (colon at end means no tag) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And no tag should be extracted + if receivedTag != "" { + t.Errorf("Expected empty tag, got %s", receivedTag) + } + + // And registry should be parsed correctly + if receivedRegistry != "registry.example.com/repo:" { + t.Errorf("Expected registry 'registry.example.com/repo:', got %s", receivedRegistry) + } + }) + + t.Run("EdgeCaseMultipleColons", func(t *testing.T) { + // Given a properly configured push environment + mocks := setupPushMocks(t) + var receivedRegistry, receivedTag string + + mocks.ArtifactBuilder.PushFunc = func(registryBase string, repoName string, tag string) error { + receivedRegistry = fmt.Sprintf("%s/%s", registryBase, repoName) + receivedTag = tag + return nil + } + + // When executing with multiple colons (should extract from last one) + rootCmd.SetArgs([]string{"push", "registry.example.com:5000/repo:v1.0.0"}) + err := Execute(mocks.Controller) + + // Then no error should be returned + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And tag should be extracted from last colon + if receivedTag != "v1.0.0" { + t.Errorf("Expected tag 'v1.0.0', got %s", receivedTag) + } + + // And registry should not include the tag + if receivedRegistry != "registry.example.com:5000/repo" { + t.Errorf("Expected registry 'registry.example.com:5000/repo', got %s", receivedRegistry) + } + }) } diff --git a/pkg/bundler/artifact.go b/pkg/bundler/artifact.go index 799d17c66..dea165cfe 100644 --- a/pkg/bundler/artifact.go +++ b/pkg/bundler/artifact.go @@ -4,15 +4,16 @@ import ( "archive/tar" "bytes" "fmt" - "io" "path/filepath" "strings" "time" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" - "github.com/google/go-containerregistry/pkg/v1/stream" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/shell" ) @@ -74,7 +75,7 @@ type Artifact interface { Initialize(injector di.Injector) error AddFile(path string, content []byte) error Create(outputPath string, tag string) (string, error) - Push(registry string, tag string) error + Push(registryBase string, repoName string, tag string) error } // ============================================================================= @@ -139,7 +140,7 @@ func (a *ArtifactBuilder) AddFile(path string, content []byte) error { // Creates compressed tar.gz with all files plus generated metadata.yaml at root. // Returns the final output path of the created artifact file. func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) { - finalName, finalVersion, metadata, err := a.parseTagAndResolveMetadata(tag) + finalName, finalVersion, metadata, err := a.parseTagAndResolveMetadata("", tag) if err != nil { return "", err } @@ -158,51 +159,86 @@ func (a *ArtifactBuilder) Create(outputPath string, tag string) (string, error) return finalOutputPath, nil } -// Push creates and uploads a FluxCD-compatible OCI artifact to the specified registry entirely in-memory. -// The registry parameter should be in format "registry.example.com/namespace/name" or "registry.example.com:port/namespace/name". -// The tag parameter should be in format "name:version" to override metadata, or empty to use existing metadata. -// Creates a FluxCD-compatible OCI artifact entirely in-memory without touching the filesystem. -// Uses FluxCD-specific media types and annotations for full compatibility with FluxCD OCI repositories. -// Returns an error if registry format is invalid, tag format is incorrect, or push operation fails. -func (a *ArtifactBuilder) Push(registry string, tag string) error { - finalName, finalVersion, metadata, err := a.parseTagAndResolveMetadata(tag) +// Push uploads the artifact to an OCI registry with explicit blob handling to prevent MANIFEST_BLOB_UNKNOWN errors. +// Implements robust blob upload strategy recommended by Red Hat for resolving registry upload issues. +// Creates tarball in memory, constructs OCI image, uploads blobs explicitly, then uploads manifest. +// Uses authenticated keychain for registry access and retry backoff for resilience. +// Registry base should be the base URL (e.g., "ghcr.io/namespace"), repoName the repository name, tag the version. +func (a *ArtifactBuilder) Push(registryBase string, repoName string, tag string) error { + finalName, tagName, metadata, err := a.parseTagAndResolveMetadata(repoName, tag) if err != nil { return err } - var repoName, tagName string - if tag != "" { - parts := strings.Split(tag, ":") - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return fmt.Errorf("invalid tag format: %s (expected format: name:version)", tag) - } - repoName = parts[0] - tagName = parts[1] - } else { - repoName = finalName - tagName = finalVersion + tarballContent, err := a.createTarballInMemory(metadata) + if err != nil { + return fmt.Errorf("failed to create tarball in memory: %w", err) } - fullRepo := fmt.Sprintf("%s/%s", registry, repoName) + layer := static.NewLayer(tarballContent, types.DockerLayer) - ref, err := name.ParseReference(fmt.Sprintf("%s:%s", fullRepo, tagName)) + img, err := a.createFluxCDCompatibleImage(layer, finalName, tagName) if err != nil { - return fmt.Errorf("invalid repository reference: %w", err) + return fmt.Errorf("failed to create OCI image: %w", err) } - tarballContent, err := a.createTarballInMemory(metadata) + repoURL := fmt.Sprintf("%s/%s", registryBase, repoName) + if tagName != "" { + repoURL = fmt.Sprintf("%s:%s", repoURL, tagName) + } + + ref, err := name.ParseReference(repoURL) if err != nil { - return fmt.Errorf("failed to create tarball in memory: %w", err) + return fmt.Errorf("failed to parse repository reference: %w", err) } - layer := stream.NewLayer(io.NopCloser(bytes.NewReader(tarballContent))) + manifest, err := img.Manifest() + if err != nil { + return fmt.Errorf("failed to get image manifest: %w", err) + } - img, err := a.createFluxCDCompatibleImage(layer, repoName, tagName) + for _, layerDesc := range manifest.Layers { + layer, err := img.LayerByDigest(layerDesc.Digest) + if err != nil { + return fmt.Errorf("failed to get layer %s: %w", layerDesc.Digest, err) + } + + blobRef := ref.Context().Digest(layerDesc.Digest.String()) + _, err = a.shims.RemoteGet(blobRef, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + err = a.shims.RemoteWriteLayer(ref.Context(), layer, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return fmt.Errorf("failed to upload layer %s: %w", layerDesc.Digest, err) + } + } + } + + configDigest, err := img.ConfigName() if err != nil { - return fmt.Errorf("failed to create FluxCD-compatible OCI image: %w", err) + return fmt.Errorf("failed to get config digest: %w", err) + } + + configRef := ref.Context().Digest(configDigest.String()) + _, err = a.shims.RemoteGet(configRef, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + configBlob, err := img.RawConfigFile() + if err != nil { + return fmt.Errorf("failed to get config file: %w", err) + } + + configLayer := static.NewLayer(configBlob, types.DockerConfigJSON) + err = a.shims.RemoteWriteLayer(ref.Context(), configLayer, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + if err != nil { + return fmt.Errorf("failed to upload config: %w", err) + } } - err = remote.Write(ref, img) + err = a.shims.RemoteWrite(ref, img, remote.WithAuthFromKeychain(authn.DefaultKeychain), remote.WithRetryBackoff(remote.Backoff{ + Duration: 1.0, + Factor: 3.0, + Jitter: 0.1, + Steps: 5, + })) if err != nil { return fmt.Errorf("failed to push artifact to registry: %w", err) } @@ -215,20 +251,31 @@ func (a *ArtifactBuilder) Push(registry string, tag string) error { // ============================================================================= // parseTagAndResolveMetadata extracts name and version from tag parameter or metadata file and generates final metadata. -// Handles tag parsing in "name:version" format and validates both components are non-empty. +// For Create method (repoName is empty): tag can be "name:version" format or empty to use metadata.yaml +// For Push method (repoName provided): tag is version only, repoName is used as fallback name // Loads existing metadata.yaml from files if present and parses it as BlueprintMetadataInput. -// Tag parameters take precedence over metadata file values for name and version. -// Requires either tag parameters or metadata file to provide name and version. +// Tag parameter takes precedence over metadata file values for version and/or name. // Returns final name, version, and complete marshaled metadata ready for embedding in artifacts. -func (a *ArtifactBuilder) parseTagAndResolveMetadata(tag string) (string, string, []byte, error) { +func (a *ArtifactBuilder) parseTagAndResolveMetadata(repoName, tag string) (string, string, []byte, error) { var tagName, tagVersion string - if tag != "" { - parts := strings.Split(tag, ":") - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return "", "", nil, fmt.Errorf("tag must be in format 'name:version', got: %s", tag) + + if repoName == "" { + if tag != "" { + if strings.Contains(tag, ":") { + parts := strings.Split(tag, ":") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", "", nil, fmt.Errorf("tag must be in format 'name:version', got: %s", tag) + } + tagName = parts[0] + tagVersion = parts[1] + } else { + tagVersion = tag + } + } + } else { + if tag != "" { + tagVersion = tag } - tagName = parts[0] - tagVersion = parts[1] } metadataData, hasMetadata := a.files["_templates/metadata.yaml"] @@ -250,8 +297,16 @@ func (a *ArtifactBuilder) parseTagAndResolveMetadata(tag string) (string, string finalVersion = tagVersion } + if finalName == "" && repoName != "" { + finalName = repoName + } + if finalName == "" { - return "", "", nil, fmt.Errorf("name is required: provide via tag parameter or metadata.yaml") + if repoName == "" { + return "", "", nil, fmt.Errorf("name is required: provide via tag parameter or metadata.yaml") + } else { + return "", "", nil, fmt.Errorf("name is required: provide via metadata.yaml") + } } if finalVersion == "" { return "", "", nil, fmt.Errorf("version is required: provide via tag parameter or metadata.yaml") diff --git a/pkg/bundler/artifact_test.go b/pkg/bundler/artifact_test.go index 33a1a8a78..85ae2b0c6 100644 --- a/pkg/bundler/artifact_test.go +++ b/pkg/bundler/artifact_test.go @@ -11,7 +11,9 @@ import ( "testing" "time" + "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/types" "github.com/windsorcli/cli/pkg/di" "github.com/windsorcli/cli/pkg/shell" @@ -123,8 +125,10 @@ func setupArtifactMocks(t *testing.T, opts ...*ArtifactSetupOptions) *ArtifactMo // Register shell with injector injector.Register("shell", mockShell) - // Create shims with test-friendly defaults - shims := NewShims() + // Use setupShims for consistent shim configuration + shims := setupShims(t) + + // Override specific shims for file system operations shims.Stat = func(name string) (os.FileInfo, error) { if name == "." { // Mock "." as an existing directory @@ -147,12 +151,6 @@ func setupArtifactMocks(t *testing.T, opts ...*ArtifactSetupOptions) *ArtifactMo return os.Create(fullPath) } - shims.YamlUnmarshal = func(data []byte, v any) error { - return nil - } - shims.YamlMarshal = func(data any) ([]byte, error) { - return []byte("test: yaml"), nil - } // Cleanup function t.Cleanup(func() { @@ -166,6 +164,84 @@ func setupArtifactMocks(t *testing.T, opts ...*ArtifactSetupOptions) *ArtifactMo } } +// setupShims provides common shim configurations for testing. +// This function sets up standard mocks for YAML, file operations, and image creation +// that can be reused across multiple test cases to reduce duplication. +func setupShims(t *testing.T) *Shims { + t.Helper() + + shims := NewShims() + + // Standard YAML processing mocks + shims.YamlUnmarshal = func(data []byte, v any) error { + input := v.(*BlueprintMetadataInput) + input.Name = "test" + input.Version = "1.0.0" + return nil + } + shims.YamlMarshal = func(data any) ([]byte, error) { + return []byte("test: yaml"), nil + } + + // Standard image creation mocks + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: []v1.Descriptor{ + { + Digest: v1.Hash{ + Algorithm: "sha256", + Hex: "abc123", + }, + Size: 1000, + }, + }, + }, nil + }, + layerByDigestFunc: func(hash v1.Hash) (v1.Layer, error) { + return &mockLayer{}, nil + }, + configNameFunc: func() (v1.Hash, error) { + return v1.Hash{ + Algorithm: "sha256", + Hex: "config123", + }, nil + }, + rawConfigFileFunc: func() ([]byte, error) { + return []byte("config"), nil + }, + } + + shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return mockImg, nil + } + shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg + } + + // Remote operations mocks for network testing + shims.RemoteGet = func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) { + return nil, fmt.Errorf("blob not found") + } + shims.RemoteWriteLayer = func(repo name.Repository, layer v1.Layer, options ...remote.Option) error { + return nil + } + shims.RemoteWrite = func(ref name.Reference, img v1.Image, options ...remote.Option) error { + return nil + } + + return shims +} + // ============================================================================= // Test Constructor // ============================================================================= @@ -435,18 +511,12 @@ description: A test project // When creating artifact with invalid tag format invalidTags := []string{ - "notag", "only:colon:", ":missingname", "missingversion:", - "", } for _, tag := range invalidTags { - if tag == "" { - continue // Skip empty tag as it's handled differently - } - _, err := builder.Create(".", tag) // Then an error should be returned @@ -780,24 +850,23 @@ func TestArtifactBuilder_Push(t *testing.T) { t.Fatalf("Failed to create artifact: %v", err) } - // When pushing with invalid tag format + // When pushing with invalid tag format (only colon-separated tags that are malformed) invalidTags := []string{ - "notag", - "only:colon:", - ":missingname", - "missingversion:", + "only:colon:", // has colon but empty version + ":missingname", // has colon but empty name + "missingversion:", // has colon but empty version } for _, tag := range invalidTags { - err := builder.Push("registry.example.com", tag) - if err == nil || !strings.Contains(err.Error(), "tag must be in format") { - t.Errorf("Expected tag format error for %s, got: %v", tag, err) + err := builder.Push("registry.example.com", "test", tag) + if err == nil || !strings.Contains(err.Error(), "failed to parse repository reference") { + t.Errorf("Expected repository reference error for %s, got: %v", tag, err) } } }) t.Run("ErrorWhenNameMissing", func(t *testing.T) { - // Given a builder with no metadata and no tag provided + // Given a builder with no metadata and no repoName provided (simulating Create method usage) builder, mocks := setup(t) // Set up mocks for metadata parsing that returns empty metadata @@ -807,8 +876,8 @@ func TestArtifactBuilder_Push(t *testing.T) { } builder.AddFile("_templates/metadata.yaml", []byte("")) - // When pushing without providing tag - err := builder.Push("registry.example.com", "") + // When pushing with empty repoName (simulates Create method scenario) + err := builder.Push("registry.example.com", "", "") if err == nil || !strings.Contains(err.Error(), "name is required") { t.Errorf("Expected name required error, got: %v", err) } @@ -828,7 +897,7 @@ func TestArtifactBuilder_Push(t *testing.T) { builder.AddFile("_templates/metadata.yaml", []byte("name: test")) // When pushing without providing tag - err := builder.Push("registry.example.com", "") + err := builder.Push("registry.example.com", "test", "") if err == nil || !strings.Contains(err.Error(), "version is required") { t.Errorf("Expected version required error, got: %v", err) } @@ -838,13 +907,7 @@ func TestArtifactBuilder_Push(t *testing.T) { // Given a builder with valid metadata builder, mocks := setup(t) - // Set up mocks for successful in-memory operation - mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { - input := v.(*BlueprintMetadataInput) - input.Name = "test" - input.Version = "1.0.0" - return nil - } + // Set up custom AppendLayers behavior for testing mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { return nil, fmt.Errorf("mock implementation error") } @@ -852,7 +915,7 @@ func TestArtifactBuilder_Push(t *testing.T) { builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) // When pushing - err := builder.Push("registry.example.com", "myapp:2.0.0") + err := builder.Push("registry.example.com", "myapp", "2.0.0") // Then should get mock implementation error (indicating it reached the OCI creation step) if err == nil || !strings.Contains(err.Error(), "mock implementation error") { @@ -864,17 +927,6 @@ func TestArtifactBuilder_Push(t *testing.T) { // Given a builder with valid metadata builder, mocks := setup(t) - // Set up mocks for successful in-memory operation - mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { - input := v.(*BlueprintMetadataInput) - input.Name = "test" - input.Version = "1.0.0" - return nil - } - mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { - return []byte("test: yaml"), nil - } - // Mock the tarball creation to verify in-memory operation tarballCreated := false mocks.Shims.NewGzipWriter = func(w io.Writer) *gzip.Writer { @@ -894,7 +946,7 @@ func TestArtifactBuilder_Push(t *testing.T) { builder.AddFile("test.txt", []byte("test content")) // When pushing (this should work entirely in-memory) - err := builder.Push("registry.example.com", "test:1.0.0") + err := builder.Push("registry.example.com", "test", "1.0.0") // Then should get expected test termination (proving in-memory operation worked) if err == nil || !strings.Contains(err.Error(), "expected test termination") { @@ -907,6 +959,30 @@ func TestArtifactBuilder_Push(t *testing.T) { } }) + t.Run("ErrorFromCreateTarballInMemory", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Mock tar writer to fail on WriteHeader + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + writeHeaderFunc: func(*tar.Header) error { + return fmt.Errorf("tar writer header failed") + }, + } + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then should get tarball creation error + if err == nil || !strings.Contains(err.Error(), "failed to create tarball in memory") { + t.Errorf("Expected tarball creation error, got: %v", err) + } + }) + t.Run("ErrorWithInvalidRepositoryReference", func(t *testing.T) { // Given a builder with valid metadata builder, mocks := setup(t) @@ -920,15 +996,15 @@ func TestArtifactBuilder_Push(t *testing.T) { builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) // When pushing with invalid registry format (contains invalid characters) - err := builder.Push("invalid registry format with spaces", "test:1.0.0") + err := builder.Push("invalid registry format with spaces", "test", "1.0.0") // Then should get repository reference error - if err == nil || !strings.Contains(err.Error(), "invalid repository reference") { + if err == nil || !strings.Contains(err.Error(), "failed to parse repository reference") { t.Errorf("Expected invalid repository reference error, got: %v", err) } }) - t.Run("ErrorFromCreateTarballInMemory", func(t *testing.T) { + t.Run("ErrorFromCreateFluxCDCompatibleImage", func(t *testing.T) { // Given a builder with valid metadata builder, mocks := setup(t) @@ -938,82 +1014,472 @@ func TestArtifactBuilder_Push(t *testing.T) { input.Version = "1.0.0" return nil } + mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { + return []byte("test: yaml"), nil + } - // Mock tar writer to fail on WriteHeader - mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { - return &mockTarWriter{ - writeHeaderFunc: func(*tar.Header) error { - return fmt.Errorf("tar writer header failed") - }, - } + // Mock AppendLayers to fail + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("config file mutation failed") } builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) // When pushing - err := builder.Push("registry.example.com", "test:1.0.0") + err := builder.Push("registry.example.com", "test", "1.0.0") - // Then should get tarball creation error - if err == nil || !strings.Contains(err.Error(), "failed to create tarball in memory") { - t.Errorf("Expected tarball creation error, got: %v", err) + // Then should get OCI image creation error + if err == nil || !strings.Contains(err.Error(), "failed to create OCI image") { + t.Errorf("Expected OCI image creation error, got: %v", err) } }) - t.Run("ErrorFromCreateFluxCDCompatibleImage", func(t *testing.T) { + t.Run("SuccessWithEmptyTag", func(t *testing.T) { // Given a builder with valid metadata builder, mocks := setup(t) - mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { - input := v.(*BlueprintMetadataInput) - input.Name = "test" - input.Version = "1.0.0" - return nil + // Set up standard mocks + mocks.Shims = setupShims(t) + builder.shims = mocks.Shims // Update builder to use new shims + + // Mock that terminates early to avoid nil pointer issues + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("expected test termination") } - mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { - return []byte("test: yaml"), nil + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing with empty tag (should use version from metadata) + err := builder.Push("registry.example.com", "test", "") + + // Then should get expected test termination (verifying it gets to image creation step) + if err == nil || !strings.Contains(err.Error(), "expected test termination") { + t.Errorf("Expected test termination error, got: %v", err) } + }) - // Mock AppendLayers to fail + t.Run("ErrorFromImageManifest", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up standard mocks with custom manifest behavior + mocks.Shims = setupShims(t) + builder.shims = mocks.Shims // Update builder to use new shims + + // Mock successful image creation but failing manifest + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + return nil, fmt.Errorf("manifest generation failed") + }, + } mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { - return nil, fmt.Errorf("config file mutation failed") + return mockImg, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg } builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) // When pushing - err := builder.Push("registry.example.com", "test:1.0.0") + err := builder.Push("registry.example.com", "test", "1.0.0") - // Then should get FluxCD image creation error - if err == nil || !strings.Contains(err.Error(), "failed to create FluxCD-compatible OCI image") { - t.Errorf("Expected FluxCD image creation error, got: %v", err) + // Then should get manifest error + if err == nil || !strings.Contains(err.Error(), "failed to get image manifest") { + t.Errorf("Expected manifest error, got: %v", err) } }) - t.Run("SuccessWithEmptyTag", func(t *testing.T) { - // Given a builder with valid metadata file + t.Run("ErrorFromLayerByDigest", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up standard mocks with custom layer behavior + mocks.Shims = setupShims(t) + builder.shims = mocks.Shims // Update builder to use new shims + + // Mock image with manifest but failing layer access + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: []v1.Descriptor{ + { + Digest: v1.Hash{ + Algorithm: "sha256", + Hex: "abc123", + }, + Size: 1000, + }, + }, + }, nil + }, + layerByDigestFunc: func(hash v1.Hash) (v1.Layer, error) { + return nil, fmt.Errorf("layer access failed") + }, + } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then should get layer access error + if err == nil || !strings.Contains(err.Error(), "failed to get layer") { + t.Errorf("Expected layer access error, got: %v", err) + } + }) + + t.Run("ErrorFromConfigName", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up standard mocks with custom config behavior + mocks.Shims = setupShims(t) + builder.shims = mocks.Shims // Update builder to use new shims + + // Mock image with empty manifest but failing config name + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: []v1.Descriptor{}, // Empty layers to skip layer upload + }, nil + }, + configNameFunc: func() (v1.Hash, error) { + return v1.Hash{}, fmt.Errorf("config name failed") + }, + } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then should get config digest error + if err == nil || !strings.Contains(err.Error(), "failed to get config digest") { + t.Errorf("Expected config digest error, got: %v", err) + } + }) + + t.Run("ErrorFromRawConfigFile", func(t *testing.T) { + // Given a builder with valid metadata + builder, mocks := setup(t) + + // Set up standard mocks with custom config file behavior + mocks.Shims = setupShims(t) + builder.shims = mocks.Shims // Update builder to use new shims + + // Mock image with successful config name but failing raw config + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: []v1.Descriptor{}, // Empty layers to skip layer upload + }, nil + }, + configNameFunc: func() (v1.Hash, error) { + return v1.Hash{ + Algorithm: "sha256", + Hex: "def456", + }, nil + }, + rawConfigFileFunc: func() ([]byte, error) { + return nil, fmt.Errorf("raw config failed") + }, + } + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg + } + + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) + + // When pushing (assuming remote.Get will fail for config, triggering upload path) + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then should get config file error + if err == nil || !strings.Contains(err.Error(), "failed to get config file") { + t.Errorf("Expected config file error, got: %v", err) + } + }) + + // Edge Cases for parseTagAndResolveMetadata Coverage + t.Run("EdgeCaseWithMultipleColonTag", func(t *testing.T) { + // Given a builder with metadata + builder, mocks := setup(t) + + // Set up YAML unmarshal to return empty values + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + // Return empty input so tag parsing is tested + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("")) + + // When creating with tag containing multiple colons (should fail in Create method) + _, err := builder.Create("test.tar.gz", "name:version:extra") + + // Then should get tag format error + if err == nil || !strings.Contains(err.Error(), "tag must be in format 'name:version'") { + t.Errorf("Expected tag format error, got: %v", err) + } + }) + + t.Run("EdgeCaseWithEmptyTagParts", func(t *testing.T) { + // Given a builder with metadata builder, mocks := setup(t) + // Set up YAML unmarshal to return empty values + mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { + return nil + } + builder.AddFile("_templates/metadata.yaml", []byte("")) + + // When creating with tag having empty parts (should fail in Create method) + invalidTags := []string{":version", "name:", ":"} + for _, tag := range invalidTags { + _, err := builder.Create("test.tar.gz", tag) + if err == nil || !strings.Contains(err.Error(), "tag must be in format 'name:version'") { + t.Errorf("Expected tag format error for '%s', got: %v", tag, err) + } + } + }) + + t.Run("EdgeCaseWithRepoNameFallback", func(t *testing.T) { + // Given a builder with metadata that has no name + builder, mocks := setup(t) + + // Set up YAML unmarshal to return metadata without name mocks.Shims.YamlUnmarshal = func(data []byte, v any) error { input := v.(*BlueprintMetadataInput) - input.Name = "myapp" - input.Version = "2.0.0" + // No name set, should fall back to repoName + input.Version = "1.0.0" return nil } - mocks.Shims.YamlMarshal = func(data any) ([]byte, error) { - return []byte("test: yaml"), nil + builder.AddFile("_templates/metadata.yaml", []byte("version: 1.0.0")) + + // Mock to terminate early after metadata resolution + mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { + return nil, fmt.Errorf("test termination - repoName fallback worked") + } + + // When pushing (should use repoName as name fallback) + err := builder.Push("registry.example.com", "fallback-name", "2.0.0") + + // Then should reach AppendLayers (proving repoName fallback worked) + if err == nil || !strings.Contains(err.Error(), "test termination - repoName fallback worked") { + t.Errorf("Expected repoName fallback to work, got: %v", err) + } + }) + + // Push Method Additional Coverage Tests + t.Run("SuccessPathWithEmptyTagName", func(t *testing.T) { + // Given a builder with valid metadata but no tag + builder, mocks := setup(t) + + // Mock to terminate early after URL construction + mockImg := &mockImageWithManifest{ + manifestFunc: func() (*v1.Manifest, error) { + // This tests the empty tagName path (repoURL without tag) + return nil, fmt.Errorf("test termination after URL construction") + }, } + + // Override image creation to return our mock mocks.Shims.AppendLayers = func(base v1.Image, layers ...v1.Layer) (v1.Image, error) { - return nil, fmt.Errorf("expected test termination") + return mockImg, nil + } + mocks.Shims.ConfigFile = func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) { + return mockImg, nil + } + mocks.Shims.MediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.ConfigMediaType = func(img v1.Image, mt types.MediaType) v1.Image { + return mockImg + } + mocks.Shims.Annotations = func(img v1.Image, annotations map[string]string) v1.Image { + return mockImg } - builder.AddFile("_templates/metadata.yaml", []byte("name: myapp\nversion: 2.0.0")) + builder.AddFile("_templates/metadata.yaml", []byte("name: test\nversion: 1.0.0")) - // When pushing with empty tag (should use metadata values) - err := builder.Push("registry.example.com", "") + // When pushing with empty tag (should construct URL without tag) + err := builder.Push("registry.example.com", "test", "") - // Then should use metadata name/version and reach the expected termination - if err == nil || !strings.Contains(err.Error(), "expected test termination") { - t.Errorf("Expected test termination error, got: %v", err) + // Then should get expected test termination + if err == nil || !strings.Contains(err.Error(), "test termination after URL construction") { + t.Errorf("Expected URL construction termination, got: %v", err) + } + }) + + t.Run("ErrorFromRemoteWriteLayer", func(t *testing.T) { + // Given a builder with files and metadata + builder, mocks := setup(t) + + // And RemoteWriteLayer fails + mocks.Shims.RemoteWriteLayer = func(repo name.Repository, layer v1.Layer, options ...remote.Option) error { + return fmt.Errorf("layer upload failed") + } + + builder.AddFile("file.txt", []byte("content")) + + // When calling Push + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then an error should be returned + if err == nil { + t.Error("Expected error from layer upload failure") + } + if !strings.Contains(err.Error(), "failed to upload layer") { + t.Errorf("Expected layer upload error, got: %v", err) + } + }) + + t.Run("ErrorFromRemoteWrite", func(t *testing.T) { + // Given a builder with files and metadata + builder, mocks := setup(t) + + // And RemoteWrite fails + mocks.Shims.RemoteWrite = func(ref name.Reference, img v1.Image, options ...remote.Option) error { + return fmt.Errorf("manifest upload failed") + } + + builder.AddFile("file.txt", []byte("content")) + + // When calling Push + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then an error should be returned + if err == nil { + t.Error("Expected error from manifest upload failure") + } + if !strings.Contains(err.Error(), "failed to push artifact to registry") { + t.Errorf("Expected manifest upload error, got: %v", err) + } + }) + + t.Run("SuccessWithBlobsExisting", func(t *testing.T) { + // Given a builder with files and metadata + builder, mocks := setup(t) + + // And RemoteGet succeeds (blobs exist) + mocks.Shims.RemoteGet = func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) { + return &remote.Descriptor{}, nil + } + + builder.AddFile("file.txt", []byte("content")) + + // When calling Push + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then no error should be returned + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("SuccessWithBlobsNotExisting", func(t *testing.T) { + // Given a builder with files and metadata + builder, mocks := setup(t) + + // And RemoteGet fails (blobs don't exist, need upload) + mocks.Shims.RemoteGet = func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) { + return nil, fmt.Errorf("blob not found") + } + // And RemoteWriteLayer succeeds + mocks.Shims.RemoteWriteLayer = func(repo name.Repository, layer v1.Layer, options ...remote.Option) error { + return nil + } + + builder.AddFile("file.txt", []byte("content")) + + // When calling Push + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then no error should be returned + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + }) + + t.Run("ErrorFromConfigUpload", func(t *testing.T) { + // Given a builder with files and metadata + builder, mocks := setup(t) + + // And config blob doesn't exist but upload fails + callCount := 0 + mocks.Shims.RemoteGet = func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) { + return nil, fmt.Errorf("blob not found") + } + mocks.Shims.RemoteWriteLayer = func(repo name.Repository, layer v1.Layer, options ...remote.Option) error { + callCount++ + if callCount == 1 { + // First call (layer upload) succeeds + return nil + } + // Second call (config upload) fails + return fmt.Errorf("config upload failed") + } + + builder.AddFile("file.txt", []byte("content")) + + // When calling Push + err := builder.Push("registry.example.com", "test", "1.0.0") + + // Then an error should be returned + if err == nil { + t.Error("Expected error from config upload failure") + } + if !strings.Contains(err.Error(), "failed to upload config") { + t.Errorf("Expected config upload error, got: %v", err) } }) } @@ -1671,3 +2137,61 @@ func (m *mockImage) Manifest() (*v1.Manifest, error) { return nil, nil } func (m *mockImage) RawManifest() ([]byte, error) { return nil, nil } func (m *mockImage) LayerByDigest(v1.Hash) (v1.Layer, error) { return nil, nil } func (m *mockImage) LayerByDiffID(v1.Hash) (v1.Layer, error) { return nil, nil } + +// Enhanced mock image with configurable behavior for testing different scenarios +type mockImageWithManifest struct { + manifestFunc func() (*v1.Manifest, error) + layerByDigestFunc func(v1.Hash) (v1.Layer, error) + configNameFunc func() (v1.Hash, error) + rawConfigFileFunc func() ([]byte, error) +} + +func (m *mockImageWithManifest) Layers() ([]v1.Layer, error) { return nil, nil } +func (m *mockImageWithManifest) MediaType() (types.MediaType, error) { return "", nil } +func (m *mockImageWithManifest) Size() (int64, error) { return 0, nil } +func (m *mockImageWithManifest) ConfigFile() (*v1.ConfigFile, error) { return nil, nil } +func (m *mockImageWithManifest) Digest() (v1.Hash, error) { return v1.Hash{}, nil } +func (m *mockImageWithManifest) RawManifest() ([]byte, error) { return nil, nil } +func (m *mockImageWithManifest) LayerByDiffID(v1.Hash) (v1.Layer, error) { return nil, nil } + +func (m *mockImageWithManifest) Manifest() (*v1.Manifest, error) { + if m.manifestFunc != nil { + return m.manifestFunc() + } + mockImg := &mockImage{} + return mockImg.Manifest() +} + +func (m *mockImageWithManifest) LayerByDigest(hash v1.Hash) (v1.Layer, error) { + if m.layerByDigestFunc != nil { + return m.layerByDigestFunc(hash) + } + mockImg := &mockImage{} + return mockImg.LayerByDigest(hash) +} + +func (m *mockImageWithManifest) ConfigName() (v1.Hash, error) { + if m.configNameFunc != nil { + return m.configNameFunc() + } + mockImg := &mockImage{} + return mockImg.ConfigName() +} + +func (m *mockImageWithManifest) RawConfigFile() ([]byte, error) { + if m.rawConfigFileFunc != nil { + return m.rawConfigFileFunc() + } + mockImg := &mockImage{} + return mockImg.RawConfigFile() +} + +// Mock layer for testing +type mockLayer struct{} + +func (m *mockLayer) Digest() (v1.Hash, error) { return v1.Hash{}, nil } +func (m *mockLayer) DiffID() (v1.Hash, error) { return v1.Hash{}, nil } +func (m *mockLayer) Compressed() (io.ReadCloser, error) { return nil, nil } +func (m *mockLayer) Uncompressed() (io.ReadCloser, error) { return nil, nil } +func (m *mockLayer) Size() (int64, error) { return 0, nil } +func (m *mockLayer) MediaType() (types.MediaType, error) { return "", nil } diff --git a/pkg/bundler/mock_artifact.go b/pkg/bundler/mock_artifact.go index 48b87f27e..a870f60b7 100644 --- a/pkg/bundler/mock_artifact.go +++ b/pkg/bundler/mock_artifact.go @@ -18,7 +18,7 @@ type MockArtifact struct { InitializeFunc func(injector di.Injector) error AddFileFunc func(path string, content []byte) error CreateFunc func(outputPath string, tag string) (string, error) - PushFunc func(registry string, tag string) error + PushFunc func(registryBase string, repoName string, tag string) error } // ============================================================================= @@ -59,9 +59,9 @@ func (m *MockArtifact) Create(outputPath string, tag string) (string, error) { } // Push calls the mock PushFunc if set, otherwise returns nil -func (m *MockArtifact) Push(registry string, tag string) error { +func (m *MockArtifact) Push(registryBase string, repoName string, tag string) error { if m.PushFunc != nil { - return m.PushFunc(registry, tag) + return m.PushFunc(registryBase, repoName, tag) } return nil } diff --git a/pkg/bundler/mock_artifact_test.go b/pkg/bundler/mock_artifact_test.go index d081b814d..b9a05efaa 100644 --- a/pkg/bundler/mock_artifact_test.go +++ b/pkg/bundler/mock_artifact_test.go @@ -143,26 +143,30 @@ func TestMockArtifact_Push(t *testing.T) { // Given a mock with a custom push function mock := NewMockArtifact() called := false - var capturedRegistry, capturedTag string - mock.PushFunc = func(registry string, tag string) error { + var capturedRegistryBase, capturedRepoName, capturedTag string + mock.PushFunc = func(registryBase string, repoName string, tag string) error { called = true - capturedRegistry = registry + capturedRegistryBase = registryBase + capturedRepoName = repoName capturedTag = tag return nil } // When calling Push - err := mock.Push("registry.example.com", "myapp:v1.0.0") + err := mock.Push("registry.example.com", "myapp", "v1.0.0") // Then the mock function should be called if !called { t.Error("Expected PushFunc to be called") } - if capturedRegistry != "registry.example.com" { - t.Errorf("Expected registry 'registry.example.com', got '%s'", capturedRegistry) + if capturedRegistryBase != "registry.example.com" { + t.Errorf("Expected registryBase 'registry.example.com', got '%s'", capturedRegistryBase) } - if capturedTag != "myapp:v1.0.0" { - t.Errorf("Expected tag 'myapp:v1.0.0', got '%s'", capturedTag) + if capturedRepoName != "myapp" { + t.Errorf("Expected repoName 'myapp', got '%s'", capturedRepoName) + } + if capturedTag != "v1.0.0" { + t.Errorf("Expected tag 'v1.0.0', got '%s'", capturedTag) } if err != nil { t.Errorf("Expected nil error, got %v", err) @@ -174,7 +178,7 @@ func TestMockArtifact_Push(t *testing.T) { mock := NewMockArtifact() // When calling Push - err := mock.Push("registry.example.com", "myapp:v1.0.0") + err := mock.Push("registry.example.com", "myapp", "v1.0.0") // Then no error should be returned if err != nil { diff --git a/pkg/bundler/shims.go b/pkg/bundler/shims.go index a0c8012e7..ca19ce3b9 100644 --- a/pkg/bundler/shims.go +++ b/pkg/bundler/shims.go @@ -8,9 +8,11 @@ import ( "path/filepath" "github.com/goccy/go-yaml" + "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/types" ) @@ -32,21 +34,24 @@ type TarWriter interface { // Shims provides mockable wrappers around system and file operations type Shims struct { - Stat func(name string) (os.FileInfo, error) - Create func(name string) (io.WriteCloser, error) - ReadFile func(name string) ([]byte, error) - Walk func(root string, walkFn filepath.WalkFunc) error - NewGzipWriter func(w io.Writer) *gzip.Writer - NewTarWriter func(w io.Writer) TarWriter - YamlUnmarshal func(data []byte, v any) error - FilepathRel func(basepath, targpath string) (string, error) - YamlMarshal func(data any) ([]byte, error) - AppendLayers func(base v1.Image, layers ...v1.Layer) (v1.Image, error) - ConfigFile func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) - MediaType func(img v1.Image, mt types.MediaType) v1.Image - ConfigMediaType func(img v1.Image, mt types.MediaType) v1.Image - Annotations func(img v1.Image, anns map[string]string) v1.Image - EmptyImage func() v1.Image + Stat func(name string) (os.FileInfo, error) + Create func(name string) (io.WriteCloser, error) + ReadFile func(name string) ([]byte, error) + Walk func(root string, walkFn filepath.WalkFunc) error + NewGzipWriter func(w io.Writer) *gzip.Writer + NewTarWriter func(w io.Writer) TarWriter + YamlUnmarshal func(data []byte, v any) error + FilepathRel func(basepath, targpath string) (string, error) + YamlMarshal func(data any) ([]byte, error) + AppendLayers func(base v1.Image, layers ...v1.Layer) (v1.Image, error) + ConfigFile func(img v1.Image, cfg *v1.ConfigFile) (v1.Image, error) + MediaType func(img v1.Image, mt types.MediaType) v1.Image + ConfigMediaType func(img v1.Image, mt types.MediaType) v1.Image + Annotations func(img v1.Image, anns map[string]string) v1.Image + EmptyImage func() v1.Image + RemoteGet func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) + RemoteWriteLayer func(repo name.Repository, layer v1.Layer, options ...remote.Option) error + RemoteWrite func(ref name.Reference, img v1.Image, options ...remote.Option) error } // ============================================================================= @@ -73,6 +78,9 @@ func NewShims() *Shims { Annotations: func(img v1.Image, anns map[string]string) v1.Image { return mutate.Annotations(img, anns).(v1.Image) }, - EmptyImage: func() v1.Image { return empty.Image }, + EmptyImage: func() v1.Image { return empty.Image }, + RemoteGet: remote.Get, + RemoteWriteLayer: remote.WriteLayer, + RemoteWrite: remote.Write, } } From f7558a8336ae6bde91db06d0ed12b3ade187c854 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Wed, 2 Jul 2025 12:03:16 -0400 Subject: [PATCH 3/3] Fix gosec --- pkg/bundler/artifact.go | 8 ++++++-- pkg/bundler/artifact_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/bundler/artifact.go b/pkg/bundler/artifact.go index dea165cfe..ebd11b2e5 100644 --- a/pkg/bundler/artifact.go +++ b/pkg/bundler/artifact.go @@ -369,8 +369,12 @@ func (a *ArtifactBuilder) createTarballInMemory(metadata []byte) ([]byte, error) } } - tarWriter.Close() - gzipWriter.Close() + if err := tarWriter.Close(); err != nil { + return nil, fmt.Errorf("failed to close tar writer: %w", err) + } + if err := gzipWriter.Close(); err != nil { + return nil, fmt.Errorf("failed to close gzip writer: %w", err) + } return buf.Bytes(), nil } diff --git a/pkg/bundler/artifact_test.go b/pkg/bundler/artifact_test.go index 85ae2b0c6..0057e2eb4 100644 --- a/pkg/bundler/artifact_test.go +++ b/pkg/bundler/artifact_test.go @@ -1682,6 +1682,30 @@ func TestArtifactBuilder_createTarballInMemory(t *testing.T) { t.Errorf("Expected file content error, got: %v", err) } }) + + t.Run("ErrorWhenTarWriterCloseFails", func(t *testing.T) { + // Given a builder with files + builder, mocks := setup(t) + builder.AddFile("test.txt", []byte("content")) + + // Mock tar writer to fail on Close + mocks.Shims.NewTarWriter = func(w io.Writer) TarWriter { + return &mockTarWriter{ + closeFunc: func() error { + return fmt.Errorf("tar writer close failed") + }, + } + } + + // When creating tarball in memory + _, err := builder.createTarballInMemory([]byte("metadata")) + + // Then should get tar writer close error + if err == nil || !strings.Contains(err.Error(), "failed to close tar writer") { + t.Errorf("Expected tar writer close error, got: %v", err) + } + }) + } // =============================================================================