diff --git a/cmd/importURL.go b/cmd/importURL.go index a62eb03..7d83ea5 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -31,12 +31,8 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm Use: "import-url", Short: "import API artifacts from URL on Microcks server", Long: `import API artifacts from URL on Microcks server`, + Args: validateImportURLArgs, RunE: func(cmd *cobra.Command, args []string) error { - // Parse subcommand args first. - if len(args) == 0 { - return errors.Wrapf(errors.KindUsage, "import-url requires a argument") - } - specificationFiles := args[0] mc, _, err := newCommandClient(globalClientOpts) @@ -64,24 +60,39 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm return importURLCmd } +func validateImportURLArgs(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return errors.Wrapf(errors.KindUsage, "import-url requires a argument") + } + + for _, f := range strings.Split(args[0], ",") { + lower := strings.ToLower(f) + if !strings.HasPrefix(lower, "https://") && !strings.HasPrefix(lower, "http://") { + return errors.Wrapf( + errors.KindUsage, + "invalid artifact URL '%s': must start with http:// or https://", + f, + ) + } + } + return nil +} + func parseImportURLArg(f string) (string, bool, string) { mainArtifact := true secret := "" - // Check if URL starts with https or http - if strings.HasPrefix(f, "https://") || strings.HasPrefix(f, "http://") { - parts := strings.Split(f, ":") - n := len(parts) + parts := strings.Split(f, ":") + n := len(parts) - for i := 2; i < n; i++ { - if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { - mainArtifact = val - if i+1 < n { - secret = strings.Join(parts[i+1:], ":") - } - f = strings.Join(parts[:i], ":") - break + for i := 2; i < n; i++ { + if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { + mainArtifact = val + if i+1 < n { + secret = strings.Join(parts[i+1:], ":") } + f = strings.Join(parts[:i], ":") + break } } return f, mainArtifact, secret diff --git a/cmd/importURL_test.go b/cmd/importURL_test.go index d733c9e..604896d 100644 --- a/cmd/importURL_test.go +++ b/cmd/importURL_test.go @@ -19,142 +19,158 @@ package cmd import ( "testing" + "github.com/microcks/microcks-cli/pkg/connectors" + "github.com/microcks/microcks-cli/pkg/errors" "github.com/stretchr/testify/assert" ) func TestParseImportURLArg(t *testing.T) { tests := []struct { - name string - input string - expectedURL string + name string + input string + expectedURL string expectedMainArtifact bool - expectedSecret string + expectedSecret string }{ { - name: "standard URL without suffixes", - input: "https://example.com/openapi.yaml", - expectedURL: "https://example.com/openapi.yaml", + name: "standard URL without suffixes", + input: "https://example.com/openapi.yaml", + expectedURL: "https://example.com/openapi.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact true", - input: "https://example.com/spec1.yaml:true", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact true", + input: "https://example.com/spec1.yaml:true", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact false", - input: "https://example.com/spec1.yaml:false", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact false", + input: "https://example.com/spec1.yaml:false", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: false, - expectedSecret: "", + expectedSecret: "", }, { - name: "standard URL with mainArtifact and secret", - input: "https://example.com/spec1.yaml:true:my-secret", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact and secret", + input: "https://example.com/spec1.yaml:true:my-secret", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: true, - expectedSecret: "my-secret", + expectedSecret: "my-secret", }, { - name: "standard URL with mainArtifact false and secret", - input: "https://example.com/spec1.yaml:false:my-secret-token", - expectedURL: "https://example.com/spec1.yaml", + name: "standard URL with mainArtifact false and secret", + input: "https://example.com/spec1.yaml:false:my-secret-token", + expectedURL: "https://example.com/spec1.yaml", expectedMainArtifact: false, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "URL with port and no suffixes", - input: "http://localhost:8585/spec.yaml", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and no suffixes", + input: "http://localhost:8585/spec.yaml", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port and mainArtifact true", - input: "http://localhost:8585/spec.yaml:true", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and mainArtifact true", + input: "http://localhost:8585/spec.yaml:true", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port and mainArtifact false", - input: "http://localhost:8585/spec.yaml:false", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port and mainArtifact false", + input: "http://localhost:8585/spec.yaml:false", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: false, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with port, mainArtifact and secret", - input: "http://localhost:8585/spec.yaml:true:my-secret-token", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port, mainArtifact and secret", + input: "http://localhost:8585/spec.yaml:true:my-secret-token", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: true, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "URL with port, mainArtifact false and secret", - input: "http://localhost:8585/spec.yaml:false:my-secret-token", - expectedURL: "http://localhost:8585/spec.yaml", + name: "URL with port, mainArtifact false and secret", + input: "http://localhost:8585/spec.yaml:false:my-secret-token", + expectedURL: "http://localhost:8585/spec.yaml", expectedMainArtifact: false, - expectedSecret: "my-secret-token", + expectedSecret: "my-secret-token", }, { - name: "malformed bool suffix preserves URL", - input: "http://localhost:8585/spec.yaml:tru", - expectedURL: "http://localhost:8585/spec.yaml:tru", + name: "malformed bool suffix preserves URL", + input: "http://localhost:8585/spec.yaml:tru", + expectedURL: "http://localhost:8585/spec.yaml:tru", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "malformed bool with secret preserves URL", - input: "http://localhost:8585/spec.yaml:tru:mysecret", - expectedURL: "http://localhost:8585/spec.yaml:tru:mysecret", + name: "malformed bool with secret preserves URL", + input: "http://localhost:8585/spec.yaml:tru:mysecret", + expectedURL: "http://localhost:8585/spec.yaml:tru:mysecret", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "URL with path and no port", - input: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", - expectedURL: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", + name: "URL with path and no port", + input: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", + expectedURL: "https://raw.githubusercontent.com/org/repo/main/spec.yaml", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "short URL without path", - input: "http://localhost:true", - expectedURL: "http://localhost", + name: "short URL without path", + input: "http://localhost:true", + expectedURL: "http://localhost", expectedMainArtifact: true, - expectedSecret: "", + expectedSecret: "", }, { - name: "boolean-like secret is not consumed as primary flag", - input: "http://example.com/api:true:false", - expectedURL: "http://example.com/api", + name: "boolean-like secret is not consumed as primary flag", + input: "http://example.com/api:true:false", + expectedURL: "http://example.com/api", expectedMainArtifact: true, - expectedSecret: "false", + expectedSecret: "false", }, { - name: "secret containing colons", - input: "http://example.com/api:true:my:secret:token", - expectedURL: "http://example.com/api", + name: "secret containing colons", + input: "http://example.com/api:true:my:secret:token", + expectedURL: "http://example.com/api", expectedMainArtifact: true, - expectedSecret: "my:secret:token", + expectedSecret: "my:secret:token", }, { - name: "URL with port and secret containing colons", - input: "http://localhost:8080/api:false:auth:basic:user:pass", - expectedURL: "http://localhost:8080/api", + name: "URL with port and secret containing colons", + input: "http://localhost:8080/api:false:auth:basic:user:pass", + expectedURL: "http://localhost:8080/api", expectedMainArtifact: false, - expectedSecret: "auth:basic:user:pass", + expectedSecret: "auth:basic:user:pass", }, { - name: "boolean host with port does not get misparsed as primary flag", - input: "http://true:8080/api:false:secret", - expectedURL: "http://true:8080/api", + name: "boolean host with port does not get misparsed as primary flag", + input: "http://true:8080/api:false:secret", + expectedURL: "http://true:8080/api", expectedMainArtifact: false, - expectedSecret: "secret", + expectedSecret: "secret", + }, + { + name: "uppercase HTTP scheme with suffix", + input: "HTTP://example.com/spec.yaml:false:mysecret", + expectedURL: "HTTP://example.com/spec.yaml", + expectedMainArtifact: false, + expectedSecret: "mysecret", + }, + { + name: "mixed-case Https scheme with suffix", + input: "Https://example.com/spec.yaml:true:mysecret", + expectedURL: "Https://example.com/spec.yaml", + expectedMainArtifact: true, + expectedSecret: "mysecret", }, } @@ -167,3 +183,106 @@ func TestParseImportURLArg(t *testing.T) { }) } } + +func TestValidateImportURLArgs(t *testing.T) { + tests := []struct { + name string + args []string + wantErr bool + expectedErrMsg string + }{ + { + name: "missing args", + args: []string{}, + wantErr: true, + expectedErrMsg: "import-url requires a argument", + }, + { + name: "valid standard URL", + args: []string{"https://example.com/spec.yaml"}, + wantErr: false, + }, + { + name: "valid uppercase HTTP scheme", + args: []string{"HTTP://example.com/spec.yaml"}, + wantErr: false, + }, + { + name: "valid mixed-case Https scheme", + args: []string{"Https://example.com/spec.yaml:false:secret"}, + wantErr: false, + }, + { + name: "relative path without scheme", + args: []string{"spec.yaml"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec.yaml': must start with http:// or https://", + }, + { + name: "relative path with suffixes without scheme", + args: []string{"spec.yaml:true:mysecret"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec.yaml:true:mysecret': must start with http:// or https://", + }, + { + name: "unsupported ftp scheme", + args: []string{"ftp://example.com/spec.yaml"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'ftp://example.com/spec.yaml': must start with http:// or https://", + }, + { + name: "file scheme", + args: []string{"file:///tmp/spec.yaml"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'file:///tmp/spec.yaml': must start with http:// or https://", + }, + { + name: "empty string", + args: []string{""}, + wantErr: true, + expectedErrMsg: "invalid artifact URL '': must start with http:// or https://", + }, + { + name: "comma-separated valid URLs", + args: []string{"https://example.com/spec1.yaml,http://example.com/spec2.yaml"}, + wantErr: false, + }, + { + name: "comma-separated with second URL invalid fails upfront", + args: []string{"https://example.com/spec1.yaml,spec2.yaml:true"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec2.yaml:true': must start with http:// or https://", + }, + { + name: "comma-separated with first URL invalid fails upfront", + args: []string{"spec1.yaml,https://example.com/spec2.yaml"}, + wantErr: true, + expectedErrMsg: "invalid artifact URL 'spec1.yaml': must start with http:// or https://", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateImportURLArgs(nil, tt.args) + if tt.wantErr { + assert.EqualError(t, err, tt.expectedErrMsg) + assert.Equal(t, errors.KindUsage, errors.KindOf(err)) + return + } + assert.NoError(t, err) + }) + } +} + +func TestImportURLCommandExecutionRejectsInvalidArgsBeforeRunE(t *testing.T) { + // Create the command with dummy client options. + // If Args validator did not reject the arguments upfront, RunE would proceed + // to newCommandClient, which would fail with a missing microcksURL connection/usage error. + cmd := NewImportURLCommand(&connectors.ClientOptions{}) + cmd.SetArgs([]string{"https://example.com/spec1.yaml,spec2.yaml:true"}) + + err := cmd.Execute() + assert.Error(t, err) + assert.Equal(t, errors.KindUsage, errors.KindOf(err)) + assert.EqualError(t, err, "invalid artifact URL 'spec2.yaml:true': must start with http:// or https://") +}