diff --git a/cmd/push.go b/cmd/push.go index d9d1e8fb5..64d5982de 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -3,11 +3,12 @@ package cmd import ( "fmt" "os" - "strings" "github.com/spf13/cobra" + "github.com/windsorcli/cli/pkg/composer" + "github.com/windsorcli/cli/pkg/composer/artifact" + "github.com/windsorcli/cli/pkg/context" "github.com/windsorcli/cli/pkg/di" - "github.com/windsorcli/cli/pkg/runtime" ) // pushCmd represents the push command @@ -31,99 +32,51 @@ Examples: windsor push registry.example.com/blueprints`, SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - // 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 registryBase, repoName, tag string - arg := args[0] + injector := cmd.Context().Value(injectorKey).(di.Injector) - // Strip oci:// prefix if present - arg = strings.TrimPrefix(arg, "oci://") + execCtx := &context.ExecutionContext{ + Injector: injector, + } - // First extract tag if present - if lastColon := strings.LastIndex(arg, ":"); lastColon > 0 && lastColon < len(arg)-1 { - // Has tag in URL format (registry/repo:tag) - tag = arg[lastColon+1:] - arg = arg[:lastColon] // Remove tag from argument + execCtx, err := context.NewContext(execCtx) + if err != nil { + return fmt.Errorf("failed to initialize context: %w", err) } - // Now extract repository name and registry base - // For URLs like "ghcr.io/windsorcli/core", we want: - // registryBase = "ghcr.io" - // repoName = "windsorcli/core" - if firstSlash := strings.Index(arg, "/"); firstSlash >= 0 { - registryBase = arg[:firstSlash] - repoName = arg[firstSlash+1:] - } else { - return fmt.Errorf("invalid registry format: must include repository path (e.g., registry.com/namespace/repo)") + composerCtx := &composer.ComposerExecutionContext{ + ExecutionContext: *execCtx, } - // Get injector from context - injector := cmd.Context().Value(injectorKey).(di.Injector) + if existingArtifactBuilder := injector.Resolve("artifactBuilder"); existingArtifactBuilder != nil { + if artifactBuilder, ok := existingArtifactBuilder.(artifact.Artifact); ok { + composerCtx.ArtifactBuilder = artifactBuilder + } + } - // Create runtime instance and push artifacts - if err := runtime.NewRuntime(&runtime.Dependencies{ - Injector: injector, - }). - LoadShell(). - ProcessArtifacts(runtime.ArtifactOptions{ - RegistryBase: registryBase, - RepoName: repoName, - Tag: tag, - OutputFunc: func(registryURL string) { - fmt.Printf("Blueprint pushed successfully: %s\n", registryURL) - }, - }). - Do(); err != nil { - if isAuthenticationError(err) { - fmt.Fprintf(os.Stderr, "Have you run 'docker login %s'?\nSee https://docs.docker.com/engine/reference/commandline/login/ for details.\n", registryBase) + comp := composer.NewComposer(composerCtx) + + registryURL, err := comp.Push(args[0]) + if err != nil { + if artifact.IsAuthenticationError(err) { + registryBase, _, _, parseErr := artifact.ParseRegistryURL(args[0]) + if parseErr == nil { + fmt.Fprintf(os.Stderr, "Have you run 'docker login %s'?\nSee https://docs.docker.com/engine/reference/commandline/login/ for details.\n", registryBase) + } return fmt.Errorf("Authentication failed") } return fmt.Errorf("failed to push artifacts: %w", err) } + fmt.Printf("Blueprint pushed successfully: %s\n", registryURL) + return nil }, } -// isAuthenticationError checks if the error is related to authentication failure -func isAuthenticationError(err error) bool { - if err == nil { - return false - } - - errStr := err.Error() - - // Common authentication error patterns - authErrorPatterns := []string{ - "UNAUTHORIZED", - "unauthorized", - "authentication required", - "authentication failed", - "not authorized", - "access denied", - "login required", - "credentials required", - "401", - "403", - "unauthenticated", - "User cannot be authenticated", - "failed to push artifact", - "POST https://", - "blobs/uploads", - } - - for _, pattern := range authErrorPatterns { - if strings.Contains(errStr, pattern) { - return true - } - } - - return false -} - func init() { rootCmd.AddCommand(pushCmd) } diff --git a/pkg/composer/artifact/artifact.go b/pkg/composer/artifact/artifact.go index 05a543a2c..abc7f4369 100644 --- a/pkg/composer/artifact/artifact.go +++ b/pkg/composer/artifact/artifact.go @@ -18,8 +18,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "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/context/shell" + "github.com/windsorcli/cli/pkg/di" ) // The ArtifactBuilder creates tar.gz artifacts from prepared build directories. @@ -1072,5 +1072,67 @@ func (a *ArtifactBuilder) downloadOCIArtifact(registry, repository, tag string) return data, nil } +// ============================================================================= +// Helper Functions +// ============================================================================= + +// ParseRegistryURL parses a registry URL string into its components. +// It handles formats like "registry.com/repo:tag", "registry.com/repo", or "oci://registry.com/repo:tag". +// Returns registryBase, repoName, tag, and an error if parsing fails. +func ParseRegistryURL(registryURL string) (registryBase, repoName, tag string, err error) { + arg := strings.TrimPrefix(registryURL, "oci://") + + if lastColon := strings.LastIndex(arg, ":"); lastColon > 0 && lastColon < len(arg)-1 { + tag = arg[lastColon+1:] + arg = arg[:lastColon] + } + + if firstSlash := strings.Index(arg, "/"); firstSlash >= 0 { + registryBase = arg[:firstSlash] + repoName = arg[firstSlash+1:] + } else { + return "", "", "", fmt.Errorf("invalid registry format: must include repository path (e.g., registry.com/namespace/repo)") + } + + return registryBase, repoName, tag, nil +} + +// IsAuthenticationError checks if the error is related to authentication failure. +// It examines common authentication error patterns in error messages to determine +// if the failure is due to authentication issues rather than other problems. +func IsAuthenticationError(err error) bool { + if err == nil { + return false + } + + errStr := err.Error() + + authErrorPatterns := []string{ + "UNAUTHORIZED", + "unauthorized", + "authentication required", + "authentication failed", + "not authorized", + "access denied", + "login required", + "credentials required", + "401", + "403", + "unauthenticated", + "User cannot be authenticated", + "failed to push artifact", + "POST https://", + "blobs/uploads", + } + + for _, pattern := range authErrorPatterns { + if strings.Contains(errStr, pattern) { + return true + } + } + + return false +} + // Ensure ArtifactBuilder implements Artifact interface var _ Artifact = (*ArtifactBuilder)(nil) diff --git a/pkg/composer/artifact/artifact_test.go b/pkg/composer/artifact/artifact_test.go index a4c44f324..c7720e9e1 100644 --- a/pkg/composer/artifact/artifact_test.go +++ b/pkg/composer/artifact/artifact_test.go @@ -3979,3 +3979,355 @@ func TestArtifactBuilder_walkAndProcessFiles(t *testing.T) { } }) } + +// ============================================================================= +// Test Helper Functions +// ============================================================================= + +func TestParseRegistryURL(t *testing.T) { + t.Run("ParsesRegistryURLWithTag", func(t *testing.T) { + // Given a registry URL with tag + url := "ghcr.io/windsorcli/core:v1.0.0" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then parsing should succeed + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And components should be correct + if registryBase != "ghcr.io" { + t.Errorf("Expected registryBase 'ghcr.io', got '%s'", registryBase) + } + if repoName != "windsorcli/core" { + t.Errorf("Expected repoName 'windsorcli/core', got '%s'", repoName) + } + if tag != "v1.0.0" { + t.Errorf("Expected tag 'v1.0.0', got '%s'", tag) + } + }) + + t.Run("ParsesRegistryURLWithoutTag", func(t *testing.T) { + // Given a registry URL without tag + url := "docker.io/myuser/myblueprint" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then parsing should succeed + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And components should be correct + if registryBase != "docker.io" { + t.Errorf("Expected registryBase 'docker.io', got '%s'", registryBase) + } + if repoName != "myuser/myblueprint" { + t.Errorf("Expected repoName 'myuser/myblueprint', got '%s'", repoName) + } + if tag != "" { + t.Errorf("Expected empty tag, got '%s'", tag) + } + }) + + t.Run("ParsesRegistryURLWithOCIPrefix", func(t *testing.T) { + // Given a registry URL with oci:// prefix + url := "oci://registry.example.com/namespace/repo:latest" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then parsing should succeed + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And prefix should be stripped + if registryBase != "registry.example.com" { + t.Errorf("Expected registryBase 'registry.example.com', got '%s'", registryBase) + } + if repoName != "namespace/repo" { + t.Errorf("Expected repoName 'namespace/repo', got '%s'", repoName) + } + if tag != "latest" { + t.Errorf("Expected tag 'latest', got '%s'", tag) + } + }) + + t.Run("ParsesRegistryURLWithMultipleSlashes", func(t *testing.T) { + // Given a registry URL with nested repository path + url := "registry.com/org/project/subproject:v2.0" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then parsing should succeed + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And full repository path should be preserved + if registryBase != "registry.com" { + t.Errorf("Expected registryBase 'registry.com', got '%s'", registryBase) + } + if repoName != "org/project/subproject" { + t.Errorf("Expected repoName 'org/project/subproject', got '%s'", repoName) + } + if tag != "v2.0" { + t.Errorf("Expected tag 'v2.0', got '%s'", tag) + } + }) + + t.Run("ReturnsErrorForInvalidFormatWithoutSlash", func(t *testing.T) { + // Given an invalid registry URL without slash + url := "registry.example.com" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then error should be returned + if err == nil { + t.Error("Expected error for invalid format, got nil") + } + + // And error should indicate invalid format + if !strings.Contains(err.Error(), "invalid registry format") { + t.Errorf("Expected error about invalid format, got: %v", err) + } + + // And components should be empty + if registryBase != "" || repoName != "" || tag != "" { + t.Errorf("Expected empty components on error, got: base=%s, repo=%s, tag=%s", registryBase, repoName, tag) + } + }) + + t.Run("ReturnsErrorForEmptyString", func(t *testing.T) { + // Given an empty URL + url := "" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then error should be returned + if err == nil { + t.Error("Expected error for empty string, got nil") + } + + // And components should be empty + if registryBase != "" || repoName != "" || tag != "" { + t.Errorf("Expected empty components on error, got: base=%s, repo=%s, tag=%s", registryBase, repoName, tag) + } + }) + + t.Run("HandlesRegistryURLWithColonInTag", func(t *testing.T) { + // Given a registry URL with multiple colons (edge case) + // The parser uses the last colon to separate repo from tag + url := "registry.com/repo:tag:with:colons" + + // When parsing the URL + registryBase, repoName, tag, err := ParseRegistryURL(url) + + // Then parsing should succeed using last colon + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + // And components should be correct (last colon is used for tag) + if registryBase != "registry.com" { + t.Errorf("Expected registryBase 'registry.com', got '%s'", registryBase) + } + if repoName != "repo:tag:with" { + t.Errorf("Expected repoName 'repo:tag:with', got '%s'", repoName) + } + if tag != "colons" { + t.Errorf("Expected tag 'colons', got '%s'", tag) + } + }) +} + +func TestIsAuthenticationError(t *testing.T) { + t.Run("ReturnsTrueForUNAUTHORIZED", func(t *testing.T) { + // Given an error with UNAUTHORIZED + err := fmt.Errorf("UNAUTHORIZED: access denied") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for UNAUTHORIZED error") + } + }) + + t.Run("ReturnsTrueForUnauthorized", func(t *testing.T) { + // Given an error with unauthorized + err := fmt.Errorf("unauthorized access") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for unauthorized error") + } + }) + + t.Run("ReturnsTrueForAuthenticationRequired", func(t *testing.T) { + // Given an error with authentication required + err := fmt.Errorf("authentication required to access this resource") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for authentication required error") + } + }) + + t.Run("ReturnsTrueForAuthenticationFailed", func(t *testing.T) { + // Given an error with authentication failed + err := fmt.Errorf("authentication failed") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for authentication failed error") + } + }) + + t.Run("ReturnsTrueForHTTP401", func(t *testing.T) { + // Given an error with HTTP 401 + err := fmt.Errorf("HTTP 401: unauthorized") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for HTTP 401 error") + } + }) + + t.Run("ReturnsTrueForHTTP403", func(t *testing.T) { + // Given an error with HTTP 403 + err := fmt.Errorf("HTTP 403: forbidden") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for HTTP 403 error") + } + }) + + t.Run("ReturnsTrueForBlobsUploads", func(t *testing.T) { + // Given an error with blobs/uploads + err := fmt.Errorf("POST https://registry.com/v2/repo/blobs/uploads: unauthorized") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for blobs/uploads error") + } + }) + + t.Run("ReturnsTrueForPOSTHTTPS", func(t *testing.T) { + // Given an error with POST https:// + err := fmt.Errorf("POST https://registry.com/v2/repo/manifests/latest: unauthorized") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for POST https:// error") + } + }) + + t.Run("ReturnsTrueForFailedToPushArtifact", func(t *testing.T) { + // Given an error with failed to push artifact + err := fmt.Errorf("failed to push artifact: unauthorized") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for failed to push artifact error") + } + }) + + t.Run("ReturnsTrueForUserCannotBeAuthenticated", func(t *testing.T) { + // Given an error with User cannot be authenticated + err := fmt.Errorf("User cannot be authenticated") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return true + if !result { + t.Error("Expected true for User cannot be authenticated error") + } + }) + + t.Run("ReturnsFalseForNilError", func(t *testing.T) { + // Given a nil error + var err error + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return false + if result { + t.Error("Expected false for nil error") + } + }) + + t.Run("ReturnsFalseForGenericError", func(t *testing.T) { + // Given a generic error + err := fmt.Errorf("network timeout") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return false + if result { + t.Error("Expected false for generic error") + } + }) + + t.Run("ReturnsFalseForParseError", func(t *testing.T) { + // Given a parse error + err := fmt.Errorf("failed to parse JSON") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return false + if result { + t.Error("Expected false for parse error") + } + }) + + t.Run("ReturnsFalseForNotFoundError", func(t *testing.T) { + // Given a not found error + err := fmt.Errorf("resource not found") + + // When checking if it's an authentication error + result := IsAuthenticationError(err) + + // Then it should return false + if result { + t.Error("Expected false for not found error") + } + }) +} diff --git a/pkg/composer/composer.go b/pkg/composer/composer.go index ca3d24d6b..de1c2fcb2 100644 --- a/pkg/composer/composer.go +++ b/pkg/composer/composer.go @@ -85,8 +85,14 @@ func (r *Composer) Bundle(outputPath, tag string) (string, error) { // Push creates and pushes an artifact to a container registry. // It bundles all project files and pushes them to the specified registry with the given tag. +// The registryURL can be in formats like "registry.com/repo:tag", "registry.com/repo", or "oci://registry.com/repo:tag". // Returns the registry URL or an error. -func (r *Composer) Push(registryBase, repoName, tag string) (string, error) { +func (r *Composer) Push(registryURL string) (string, error) { + registryBase, repoName, tag, err := artifact.ParseRegistryURL(registryURL) + if err != nil { + return "", fmt.Errorf("failed to parse registry URL: %w", err) + } + if err := r.ArtifactBuilder.Initialize(r.Injector); err != nil { return "", fmt.Errorf("failed to initialize artifact builder: %w", err) } @@ -99,12 +105,12 @@ func (r *Composer) Push(registryBase, repoName, tag string) (string, error) { return "", fmt.Errorf("failed to push artifact: %w", err) } - registryURL := fmt.Sprintf("%s/%s", registryBase, repoName) + resultURL := fmt.Sprintf("%s/%s", registryBase, repoName) if tag != "" { - registryURL = fmt.Sprintf("%s:%s", registryURL, tag) + resultURL = fmt.Sprintf("%s:%s", resultURL, tag) } - return registryURL, nil + return resultURL, nil } // Generate processes and deploys the complete project infrastructure. @@ -139,14 +145,3 @@ func (r *Composer) Generate(overwrite ...bool) error { return nil } - -// ============================================================================= -// Helper Functions -// ============================================================================= - -// CreateComposer creates a new Composer instance with all dependencies properly initialized. -// This is a convenience function that creates a fully configured Composer -// with the provided execution context. -func CreateComposer(ctx *ComposerExecutionContext) *Composer { - return NewComposer(ctx) -} diff --git a/pkg/composer/composer_test.go b/pkg/composer/composer_test.go index 8a047607d..3e5c9b156 100644 --- a/pkg/composer/composer_test.go +++ b/pkg/composer/composer_test.go @@ -3,10 +3,10 @@ package composer import ( "testing" - "github.com/windsorcli/cli/pkg/context/config" "github.com/windsorcli/cli/pkg/context" - "github.com/windsorcli/cli/pkg/di" + "github.com/windsorcli/cli/pkg/context/config" "github.com/windsorcli/cli/pkg/context/shell" + "github.com/windsorcli/cli/pkg/di" ) // ============================================================================= @@ -38,18 +38,18 @@ func setupComposerMocks(t *testing.T) *Mocks { } return &Mocks{ - Injector: injector, - ConfigHandler: configHandler, - Shell: shell, + Injector: injector, + ConfigHandler: configHandler, + Shell: shell, ComposerExecutionContext: composerCtx, } } // Mocks contains all the mock dependencies for testing type Mocks struct { - Injector di.Injector - ConfigHandler config.ConfigHandler - Shell shell.Shell + Injector di.Injector + ConfigHandler config.ConfigHandler + Shell shell.Shell ComposerExecutionContext *ComposerExecutionContext } @@ -97,7 +97,7 @@ func TestCreateComposer(t *testing.T) { t.Run("CreatesComposerWithDependencies", func(t *testing.T) { mocks := setupComposerMocks(t) - composer := CreateComposer(mocks.ComposerExecutionContext) + composer := NewComposer(mocks.ComposerExecutionContext) if composer == nil { t.Fatal("Expected Composer to be created") @@ -143,7 +143,7 @@ func TestComposer_Push(t *testing.T) { // This test would need proper mocking of the artifact builder // For now, we'll just test that the method exists and handles errors - _, err := composer.Push("ghcr.io", "test/repo", "latest") + _, err := composer.Push("ghcr.io/test/repo:latest") // We expect an error here because we don't have proper mocks set up if err == nil { t.Error("Expected error due to missing mocks, but got nil")