diff --git a/pkg/config/config.go b/pkg/config/config.go index 617eea2..96c131b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -49,6 +49,35 @@ type OperatorConfig struct { // command for running the operator, just for local operator, default is "make deploy" Command string `yaml:"command,omitempty"` + + // Violet configures how the operatorhub flow invokes the external `violet` + // binary to create Artifact / ArtifactVersion CRs. When nil, the legacy + // in-process path is used (kept for the local operator and for transitional + // builds; will be removed once violet path is the only one). + Violet *VioletConfig `yaml:"violet,omitempty"` +} + +// VioletConfig captures everything upgrade CLI needs to invoke `violet push` +// for the operatorhub onboarding flow. +type VioletConfig struct { + // Bin is an optional absolute path to the violet binary. Empty falls back + // to looking up `violet` in $PATH. + Bin string `yaml:"bin,omitempty"` + + // PackagePrefix is the MinIO (or HTTP) root from which the per-version + // .tgz is downloaded. Default: "http://package-minio.alauda.cn:9199/packages/". + PackagePrefix string `yaml:"packagePrefix,omitempty"` + + // SkipPush controls whether `--skip-push` is passed to `violet push`. + // Pointer so we can distinguish "unset" (treated as true) from "explicit + // false" (private-registry scenario that wants violet to also push images). + SkipPush *bool `yaml:"skipPush,omitempty"` + + // PushArgs are extra arguments appended verbatim to `violet push`, used to + // inject options such as --dest-repo, --plain, --image-pull-secret, --force + // in private-registry scenarios. Credentials must come from environment + // variables (VIOLET_REGISTRY_USERNAME / VIOLET_REGISTRY_PASSWORD), not here. + PushArgs []string `yaml:"pushArgs,omitempty"` } // UpgradePath represents a single upgrade path @@ -71,8 +100,16 @@ type Version struct { TestSubPath string `yaml:"testSubPath,omitempty"` // revision is the revision to use for the version Channel string `yaml:"channel,omitempty"` + // ExpectedSha256, when non-empty, is the lowercase hex SHA-256 of the + // downloaded .tgz. The upgrade CLI verifies the digest after download and + // fails fast on mismatch. Optional; leave empty to skip verification. + ExpectedSha256 string `yaml:"expectedSha256,omitempty"` } +// DefaultVioletPackagePrefix is the MinIO root used when OperatorConfig.Violet +// is configured but its PackagePrefix is left blank. +const DefaultVioletPackagePrefix = "http://package-minio.alauda.cn:9199/packages/" + // LoadConfig loads the configuration from a YAML file func LoadConfig(path string) (*Config, error) { data, err := os.ReadFile(path) @@ -108,5 +145,15 @@ func defaultConfig(config *Config) *Config { config.OperatorConfig.Timeout = 10 * time.Minute } + if v := config.OperatorConfig.Violet; v != nil { + if v.PackagePrefix == "" { + v.PackagePrefix = DefaultVioletPackagePrefix + } + if v.SkipPush == nil { + t := true + v.SkipPush = &t + } + } + return config } diff --git a/pkg/exec/exec.go b/pkg/exec/exec.go index 18777fa..b6dc325 100644 --- a/pkg/exec/exec.go +++ b/pkg/exec/exec.go @@ -3,9 +3,11 @@ package exec import ( "bytes" "context" + "fmt" "io" "os" "os/exec" + "strings" ) type Command struct { @@ -13,6 +15,13 @@ type Command struct { Args []string Dir string Env []string + + // EnvAllowlist limits which host environment variables are passed to the child + // process. Entries support an exact name ("KUBECONFIG") or a "PREFIX_*" pattern + // ("VIOLET_*"). When EnvAllowlist is empty, the child inherits os.Environ() in + // full (backward-compatible default). Env is always appended after the filtered + // host set. + EnvAllowlist []string } // CommandResult represents the result of a command execution @@ -37,6 +46,9 @@ func (c *Command) WithEnv(env []string) CommandOption { } } +// stderrTailLines bounds how many trailing stderr lines are wrapped into the error. +const stderrTailLines = 20 + // RunCommand executes a command and returns its stdout, stderr and error // If the command fails, it will return the error along with the captured output // The command's output will be printed to console in real-time while also being captured @@ -44,10 +56,9 @@ func RunCommand(ctx context.Context, cmd Command) CommandResult { runCmd := exec.CommandContext(ctx, cmd.Name, cmd.Args...) runCmd.Dir = cmd.Dir - // Inherit current process environment variables - runCmd.Env = os.Environ() - - // Add custom environment variables if specified + // Build child environment: allowlist-filtered host env (or full passthrough when empty), + // then append custom Env entries. + runCmd.Env = filterHostEnv(cmd.EnvAllowlist) if len(cmd.Env) > 0 { runCmd.Env = append(runCmd.Env, cmd.Env...) } @@ -65,16 +76,74 @@ func RunCommand(ctx context.Context, cmd Command) CommandResult { // Run the command err := runCmd.Run() if err != nil { - return CommandResult{ - Stdout: stdoutBuf.String(), - Stderr: stderrBuf.String(), - Err: err, - } + err = wrapWithStderrTail(err, stderrBuf.String(), stderrTailLines) } - return CommandResult{ Stdout: stdoutBuf.String(), Stderr: stderrBuf.String(), - Err: nil, + Err: err, + } +} + +// filterHostEnv returns os.Environ() filtered by allowlist. An empty allowlist +// returns the full os.Environ() (existing behaviour preserved). +func filterHostEnv(allowlist []string) []string { + host := os.Environ() + if len(allowlist) == 0 { + return host + } + out := make([]string, 0, len(host)) + for _, entry := range host { + eq := strings.IndexByte(entry, '=') + if eq < 0 { + continue + } + name := entry[:eq] + if matchAllowlist(name, allowlist) { + out = append(out, entry) + } + } + return out +} + +// matchAllowlist reports whether name matches any pattern in allowlist. A pattern +// ending in "*" matches by prefix; anything else is an exact match. +func matchAllowlist(name string, allowlist []string) bool { + for _, pat := range allowlist { + if strings.HasSuffix(pat, "*") { + if strings.HasPrefix(name, strings.TrimSuffix(pat, "*")) { + return true + } + continue + } + if name == pat { + return true + } + } + return false +} + +// wrapWithStderrTail enriches err with the trailing stderr lines so callers see +// the actionable failure context without re-reading CommandResult.Stderr. +func wrapWithStderrTail(err error, stderr string, maxLines int) error { + tail := lastLines(stderr, maxLines) + if tail == "" { + return err + } + return fmt.Errorf("%w; stderr tail:\n%s", err, tail) +} + +func lastLines(s string, n int) string { + if s == "" || n <= 0 { + return "" + } + trimmed := strings.TrimRight(s, "\n") + if trimmed == "" { + return "" + } + lines := strings.Split(trimmed, "\n") + if len(lines) <= n { + return trimmed } + return strings.Join(lines[len(lines)-n:], "\n") } diff --git a/pkg/operator/operatorhub/operator.go b/pkg/operator/operatorhub/operator.go index f3b107d..1d32818 100644 --- a/pkg/operator/operatorhub/operator.go +++ b/pkg/operator/operatorhub/operator.go @@ -26,6 +26,11 @@ type Operator struct { const ( systemNamespace = "cpaas-system" + // targetCatalogSource is the OLM catalog source the operatorhub flow targets. + // It is referenced both in CR creation (via violet --target-catalog-source) + // and by the Subscription spec, so it lives next to systemNamespace to keep + // platform-coupled constants in one place. + targetCatalogSource = "platform" ) var ( diff --git a/pkg/operator/operatorhub/violet.go b/pkg/operator/operatorhub/violet.go new file mode 100644 index 0000000..36f1e17 --- /dev/null +++ b/pkg/operator/operatorhub/violet.go @@ -0,0 +1,116 @@ +package operatorhub + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "os" + "strings" +) + +// Environment variables consumed when assembling `violet push`. They are +// intentionally not part of OperatorConfig: pipelines inject them as secrets +// and the upgrade CLI must not persist them to disk or echo them back. +const ( + EnvVioletRegistryUsername = "VIOLET_REGISTRY_USERNAME" + EnvVioletRegistryPassword = "VIOLET_REGISTRY_PASSWORD" +) + +// violet CLI flag names. Kept private so the consuming code stays expressive +// (`flagSkipPush` rather than the raw string everywhere). +const ( + flagSkipPush = "--skip-push" + flagTargetCatalogSource = "--target-catalog-source" + flagUsername = "--username" + flagPassword = "--password" +) + +// BuildPackageURL composes the .tgz URL by the agreed MinIO convention: +// +// ///.latest.ALL..tgz +// +// A trailing slash on prefix is tolerated. All four inputs are required; any +// empty value returns an error so callers fail loudly instead of producing a +// malformed URL that 404s later. +func BuildPackageURL(prefix, name, channel, bundleVersion string) (string, error) { + switch { + case prefix == "": + return "", fmt.Errorf("packagePrefix is empty") + case name == "": + return "", fmt.Errorf("operator name is empty") + case channel == "": + return "", fmt.Errorf("channel is empty (Version.Channel is required when using violet)") + case bundleVersion == "": + return "", fmt.Errorf("bundleVersion is empty") + } + p := strings.TrimRight(prefix, "/") + return fmt.Sprintf("%s/%s/%s/%s.latest.ALL.%s.tgz", p, name, channel, name, bundleVersion), nil +} + +// BuildVioletPushArgs assembles the argv for `violet push `. Credentials +// are read from EnvVioletRegistryUsername / EnvVioletRegistryPassword and +// injected as --username / --password when non-empty; pushArgs is appended +// verbatim. The function is a pure transform — it never touches the filesystem +// or starts a process — so callers can table-test the exact argv shape. +// +// The decision of skipPush belongs to the caller (it has already deferenced +// VioletConfig.SkipPush and applied the "nil == true" default in config), so +// this function takes a plain bool. +func BuildVioletPushArgs(tgzPath string, skipPush bool, pushArgs []string) []string { + args := []string{"push", tgzPath, flagTargetCatalogSource, targetCatalogSource} + if skipPush { + args = append(args, flagSkipPush) + } + if u := os.Getenv(EnvVioletRegistryUsername); u != "" { + args = append(args, flagUsername, u) + } + if p := os.Getenv(EnvVioletRegistryPassword); p != "" { + args = append(args, flagPassword, p) + } + args = append(args, pushArgs...) + return args +} + +// MaskCommand renders the command for logging, replacing the token following +// --password with `***`. This only protects log output — the credential is +// still visible to OS-level inspection (e.g. `ps auxe`) once the child process +// runs. The README must document that risk for shared CI runners. +func MaskCommand(name string, args []string) string { + parts := make([]string, 0, len(args)+1) + parts = append(parts, name) + for i := 0; i < len(args); i++ { + if args[i] == flagPassword && i+1 < len(args) { + parts = append(parts, args[i], "***") + i++ + continue + } + parts = append(parts, args[i]) + } + return strings.Join(parts, " ") +} + +// VerifySha256 streams the file at filePath and compares its SHA-256 against +// expected (hex, case-insensitive). An empty expected disables verification — +// callers can pass Version.ExpectedSha256 directly without nil-checking. The +// returned error names the path so failures are actionable without extra +// wrapping at the call site. +func VerifySha256(filePath, expected string) error { + if expected == "" { + return nil + } + f, err := os.Open(filePath) + if err != nil { + return fmt.Errorf("open %s: %w", filePath, err) + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return fmt.Errorf("read %s: %w", filePath, err) + } + actual := hex.EncodeToString(h.Sum(nil)) + if !strings.EqualFold(actual, expected) { + return fmt.Errorf("sha256 mismatch for %s: expected %s, got %s", filePath, expected, actual) + } + return nil +} diff --git a/pkg/operator/operatorhub/violet_test.go b/pkg/operator/operatorhub/violet_test.go new file mode 100644 index 0000000..66b51ef --- /dev/null +++ b/pkg/operator/operatorhub/violet_test.go @@ -0,0 +1,223 @@ +package operatorhub + +import ( + "crypto/sha256" + "encoding/hex" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBuildPackageURL(t *testing.T) { + tests := []struct { + name string + prefix string + opName string + channel string + bundleVersion string + want string + wantErr bool + }{ + { + name: "v4.6 channel with v-prefixed version", + prefix: "http://package-minio.alauda.cn:9199/packages/", + opName: "tektoncd-operator", + channel: "v4.6", + bundleVersion: "v4.6.0", + want: "http://package-minio.alauda.cn:9199/packages/tektoncd-operator/v4.6/tektoncd-operator.latest.ALL.v4.6.0.tgz", + }, + { + name: "rc channel with rc-build suffix", + prefix: "http://package-minio.alauda.cn:9199/packages/", + opName: "tektoncd-operator", + channel: "rc", + bundleVersion: "v4.2.5-rc.76.g976cff6", + want: "http://package-minio.alauda.cn:9199/packages/tektoncd-operator/rc/tektoncd-operator.latest.ALL.v4.2.5-rc.76.g976cff6.tgz", + }, + { + name: "prefix without trailing slash", + prefix: "http://example.com/pkgs", + opName: "op", + channel: "stable", + bundleVersion: "v1.0.0", + want: "http://example.com/pkgs/op/stable/op.latest.ALL.v1.0.0.tgz", + }, + {name: "empty prefix", prefix: "", opName: "op", channel: "stable", bundleVersion: "v1.0.0", wantErr: true}, + {name: "empty name", prefix: "http://x/", channel: "stable", bundleVersion: "v1.0.0", wantErr: true}, + {name: "empty channel", prefix: "http://x/", opName: "op", bundleVersion: "v1.0.0", wantErr: true}, + {name: "empty bundleVersion", prefix: "http://x/", opName: "op", channel: "stable", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := BuildPackageURL(tc.prefix, tc.opName, tc.channel, tc.bundleVersion) + if tc.wantErr { + if err == nil { + t.Fatalf("want error, got url=%q", got) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.want { + t.Errorf("url mismatch:\n got: %s\nwant: %s", got, tc.want) + } + }) + } +} + +func TestBuildVioletPushArgs(t *testing.T) { + tests := []struct { + name string + skipPush bool + pushArgs []string + envUser string + envPass string + want []string + }{ + { + name: "skip-push only, no creds, no extra args", + skipPush: true, + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform", "--skip-push"}, + }, + { + name: "skip-push false drops the flag", + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform"}, + }, + { + name: "env creds appended after skip-push", + skipPush: true, + envUser: "u", + envPass: "p", + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform", "--skip-push", "--username", "u", "--password", "p"}, + }, + { + name: "only username present (rare, but accepted)", + envUser: "u", + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform", "--username", "u"}, + }, + { + name: "private-push flow: skip-push=false + pushArgs", + pushArgs: []string{"--dest-repo", "registry.private/devops", "--plain", "--force"}, + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform", "--dest-repo", "registry.private/devops", "--plain", "--force"}, + }, + { + name: "push args land after credentials", + skipPush: true, + envUser: "u", + envPass: "p", + pushArgs: []string{"--force"}, + want: []string{"push", "/tmp/x.tgz", "--target-catalog-source", "platform", "--skip-push", "--username", "u", "--password", "p", "--force"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(EnvVioletRegistryUsername, tc.envUser) + t.Setenv(EnvVioletRegistryPassword, tc.envPass) + got := BuildVioletPushArgs("/tmp/x.tgz", tc.skipPush, tc.pushArgs) + if !stringSliceEqual(got, tc.want) { + t.Errorf("args mismatch:\n got: %v\nwant: %v", got, tc.want) + } + }) + } +} + +func TestMaskCommand(t *testing.T) { + tests := []struct { + name string + in []string + want string + }{ + { + name: "password value masked", + in: []string{"push", "x.tgz", "--username", "u", "--password", "secret", "--skip-push"}, + want: "violet push x.tgz --username u --password *** --skip-push", + }, + { + name: "no password flag is a no-op", + in: []string{"push", "x.tgz", "--skip-push"}, + want: "violet push x.tgz --skip-push", + }, + { + name: "dangling --password (no following token) is preserved verbatim", + in: []string{"push", "x.tgz", "--password"}, + want: "violet push x.tgz --password", + }, + { + name: "multiple --password occurrences each get masked", + in: []string{"--password", "a", "--password", "b"}, + want: "violet --password *** --password ***", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := MaskCommand("violet", tc.in); got != tc.want { + t.Errorf("\n got: %s\nwant: %s", got, tc.want) + } + }) + } +} + +func TestVerifySha256(t *testing.T) { + dir := t.TempDir() + fp := filepath.Join(dir, "blob") + payload := []byte("hello violet") + if err := os.WriteFile(fp, payload, 0o644); err != nil { + t.Fatal(err) + } + correct := computeSha256Hex(payload) + + t.Run("empty expected skips verification", func(t *testing.T) { + if err := VerifySha256(fp, ""); err != nil { + t.Errorf("expected nil, got %v", err) + } + }) + t.Run("correct sha passes", func(t *testing.T) { + if err := VerifySha256(fp, correct); err != nil { + t.Errorf("expected nil for correct sha, got %v", err) + } + }) + t.Run("uppercase hex still matches (case-insensitive)", func(t *testing.T) { + if err := VerifySha256(fp, strings.ToUpper(correct)); err != nil { + t.Errorf("uppercase hex should match, got %v", err) + } + }) + t.Run("wrong sha returns mismatch error", func(t *testing.T) { + err := VerifySha256(fp, "deadbeef") + if err == nil { + t.Fatal("expected mismatch error, got nil") + } + if !strings.Contains(err.Error(), "sha256 mismatch") { + t.Errorf("error should mention 'sha256 mismatch', got %v", err) + } + }) + t.Run("missing file returns open error", func(t *testing.T) { + if err := VerifySha256(filepath.Join(dir, "missing"), "abc"); err == nil { + t.Error("expected open error, got nil") + } + }) +} + +// computeSha256Hex mirrors VerifySha256's hashing so the test can assert the +// "correct sha passes" path without hardcoding a digest. +func computeSha256Hex(b []byte) string { + h := sha256.New() + h.Write(b) + return hex.EncodeToString(h.Sum(nil)) +} + +func stringSliceEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +}