From 2263be6f86e44374d7225fe52f49a6a0f98d4fc7 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Thu, 13 Aug 2026 12:46:48 +0200 Subject: [PATCH 01/14] ROX-35434: Add support for overriding image repository --- cmd/deploy.go | 26 +++++++ cmd/deploy_test.go | 50 ++++++++++++++ internal/deployer/acs_images.go | 8 +-- internal/deployer/config.go | 18 +++-- internal/deployer/deploy_via_operator.go | 28 +++----- internal/deployer/deployer.go | 2 +- internal/deployer/konflux_test.go | 9 ++- internal/deployer/operator.go | 45 ++++++++++-- .../deployer/operator_integration_test.go | 39 +++++++++++ internal/dockerauth/dockerauth.go | 68 +++++++++++-------- internal/dockerauth/dockerauth_test.go | 40 +++++++++-- tests/e2e/custom_registry_test.go | 59 ++++++++++++++++ 12 files changed, 323 insertions(+), 69 deletions(-) create mode 100644 internal/deployer/operator_integration_test.go create mode 100644 tests/e2e/custom_registry_test.go diff --git a/cmd/deploy.go b/cmd/deploy.go index f4e32866..b15e6481 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -7,13 +7,16 @@ import ( "fmt" "math/big" "os" + "strings" "time" "dario.cat/mergo" + "github.com/google/go-containerregistry/pkg/name" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/stackrox/roxie/internal/clusterdefaults" "github.com/stackrox/roxie/internal/component" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/deployer" "github.com/stackrox/roxie/internal/env" "github.com/stackrox/roxie/internal/helpers" @@ -446,6 +449,21 @@ func configureConfig(log *logger.Logger, components component.Component, deployS return nil } +// validateImageRegistry checks that registry is a well-formed "host/repository-path" string, e.g. "quay.io/rhacs-eng". +func validateImageRegistry(registry string) error { + host, repoPath, hasPath := strings.Cut(registry, "/") + if !hasPath || repoPath == "" { + return fmt.Errorf("roxie.imageRegistry must include a repository path (e.g. %s), got: %s", constants.DefaultRegistry, registry) + } + if _, err := name.NewRegistry(host); err != nil { + return fmt.Errorf("roxie.imageRegistry has an invalid registry host %q: %w", host, err) + } + if _, err := name.NewRepository(repoPath); err != nil { + return fmt.Errorf("roxie.imageRegistry has an invalid repository path %q: %w", repoPath, err) + } + return nil +} + func deployValidate(log *logger.Logger, components component.Component, deploySettings *deployer.Config) error { if components.IncludesCentral() && os.Getenv("ROXIE_SHELL") != "" { return errors.New("already in a roxie sub-shell (ROXIE_SHELL environment variable is set), please exit the shell and try again") @@ -481,10 +499,18 @@ func deployValidate(log *logger.Logger, components component.Component, deploySe return errors.New("skipping operator deployment while also requesting deploying via OLM at the same time does not make sense") } + registry := deploySettings.Roxie.Registry() + if err := validateImageRegistry(registry); err != nil { + return err + } + if deploySettings.Roxie.KonfluxImagesEnabled() { if deploySettings.Operator.DeployViaOlmEnabled() { return errors.New("using Konflux images while deploying operator via OLM is not supported") } + if registry != constants.DefaultRegistry { + return fmt.Errorf("using Konflux images with a custom image registry (%s) is not supported", registry) + } } if deploySettings.HasMixedVersions() { diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index be7976b1..a8199d8e 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -8,6 +8,7 @@ import ( "time" "dario.cat/mergo" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/deployer" "github.com/stackrox/roxie/internal/imagetag" "github.com/stackrox/roxie/internal/logger" @@ -314,6 +315,55 @@ func TestNewDeployCmd_SetRejectsSpec(t *testing.T) { } } +func TestValidateImageRegistry(t *testing.T) { + tests := []struct { + name string + registry string + expectError bool + errorContains string + }{ + {name: "default registry", registry: constants.DefaultRegistry}, + {name: "valid host/path registry", registry: "quay.io/stackrox-io"}, + {name: "registry host with port", registry: "localhost:5000/rhacs-eng"}, + { + name: "bare host with no path is rejected", + registry: "justahost", + expectError: true, + errorContains: "must include a repository path", + }, + { + name: "trailing slash with no path is rejected", + registry: "quay.io/", + expectError: true, + errorContains: "must include a repository path", + }, + { + name: "invalid registry host", + registry: "quay io/rhacs-eng", + expectError: true, + errorContains: "invalid registry host", + }, + { + name: "invalid repository path characters", + registry: "quay.io/RHACS-ENG", + expectError: true, + errorContains: "invalid repository path", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateImageRegistry(tt.registry) + if tt.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errorContains) + return + } + require.NoError(t, err) + }) + } +} + func TestApplyUserDefaults(t *testing.T) { log := logger.New() diff --git a/internal/deployer/acs_images.go b/internal/deployer/acs_images.go index 78b85805..de27c916 100644 --- a/internal/deployer/acs_images.go +++ b/internal/deployer/acs_images.go @@ -2,13 +2,11 @@ package deployer import ( "fmt" - - "github.com/stackrox/roxie/internal/constants" ) func imagesForConfig(config Config) []string { var images []string - imageRegistry := constants.DefaultRegistry + imageRegistry := config.Roxie.Registry() for _, instance := range config.OperatorInstances() { prefix := "" @@ -20,8 +18,8 @@ func imagesForConfig(config Config) []string { fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", instance.Version), fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", instance.Version), fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4", instance.Version), - instance.OperatorImage(), - instance.BundleImage(), + instance.OperatorImage(imageRegistry), + instance.BundleImage(imageRegistry), ) } diff --git a/internal/deployer/config.go b/internal/deployer/config.go index a8d5df20..988fd2ff 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -2,6 +2,7 @@ package deployer import ( "fmt" + "strings" "time" "github.com/stackrox/roxie/internal/constants" @@ -56,12 +57,22 @@ func (c *Config) DeepCopy() (*Config, error) { // RoxieConfig holds roxie-level settings such as version and feature flags. type RoxieConfig struct { Version imagetag.MainTag `yaml:"version,omitempty"` + ImageRegistry string `yaml:"imageRegistry,omitempty"` KonfluxImages *bool `yaml:"konfluxImages,omitempty"` FeatureFlags map[string]bool `yaml:"featureFlags,omitempty"` ClusterType types.ClusterType `yaml:"clusterType,omitempty"` HAProxy HAProxyConfig `yaml:"haProxy,omitempty"` } +// Registry returns the resolved image registry, defaulting to +// constants.DefaultRegistry when ImageRegistry is not set. +func (c *RoxieConfig) Registry() string { + if c.ImageRegistry == "" { + return constants.DefaultRegistry + } + return strings.TrimSuffix(c.ImageRegistry, "/") +} + func (c *RoxieConfig) KonfluxImagesSet() bool { return c.KonfluxImages != nil } @@ -118,8 +129,7 @@ func (c *OperatorInstanceConfig) ClusterRoleBindingName() string { } // BundleImage returns the operator bundle image for this operator instance. -func (c *OperatorInstanceConfig) BundleImage() string { - imageRegistry := constants.DefaultRegistry +func (c *OperatorInstanceConfig) BundleImage(imageRegistry string) string { operatorTag := c.Version.ToOperatorTag() if c.KonfluxImagesEnabled() { return fmt.Sprintf("%s/release-operator-bundle:v%s", imageRegistry, operatorTag) @@ -127,8 +137,8 @@ func (c *OperatorInstanceConfig) BundleImage() string { return fmt.Sprintf("%s/stackrox-operator-bundle:v%s", imageRegistry, operatorTag) } -func (c *OperatorInstanceConfig) OperatorImage() string { - imageRegistry := constants.DefaultRegistry +// OperatorImage returns the operator image for this operator instance. +func (c *OperatorInstanceConfig) OperatorImage(imageRegistry string) string { operatorTag := c.Version.ToOperatorTag() if c.KonfluxImagesEnabled() { return fmt.Sprintf("%s/release-operator:%s", imageRegistry, operatorTag) diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 33b7af76..323329d2 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -12,13 +12,14 @@ import ( "strings" "time" + "gopkg.in/yaml.v3" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/stackrox/roxie/internal/component" "github.com/stackrox/roxie/internal/env" "github.com/stackrox/roxie/internal/helpers" "github.com/stackrox/roxie/internal/k8s" "github.com/stackrox/roxie/internal/types" - "gopkg.in/yaml.v3" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) var ( @@ -247,7 +248,8 @@ func (d *Deployer) deployCentralOperator(ctx context.Context) error { return d.configureCentralEndpoint(ctx) } -// isOperatorVersionCorrect checks if the deployed operator matches the desired version. +// isOperatorVersionCorrect checks if the deployed operator matches the desired +// image, comparing the full reference (registry, repository, and tag). func (d *Deployer) isOperatorVersionCorrect(ctx context.Context, instance OperatorInstanceConfig) bool { currentImage, err := d.getDeployedOperatorImage(ctx, instance.Namespace) if err != nil { @@ -255,19 +257,11 @@ func (d *Deployer) isOperatorVersionCorrect(ctx context.Context, instance Operat return false } - // Extract the tag from the current image - parts := strings.SplitN(currentImage, ":", 2) - if len(parts) < 2 { - d.logger.Warningf("Could not parse operator image tag from: %s", currentImage) - return false - } - currentTag := parts[1] - - desiredTag := instance.Version.ToOperatorTag().String() - if currentTag != desiredTag { - d.logger.Info("Operator version mismatch detected:") - d.logger.Infof(" Current: %s", currentTag) - d.logger.Infof(" Desired: %s", desiredTag) + desiredImage := instance.OperatorImage(d.config.Roxie.Registry()) + if currentImage != desiredImage { + d.logger.Info("Operator image mismatch detected:") + d.logger.Infof(" Current: %s", currentImage) + d.logger.Infof(" Desired: %s", desiredImage) return false } return true @@ -309,7 +303,7 @@ func (d *Deployer) ensurePullSecretExists(ctx context.Context, namespace string) return errors.New("no pull secrets available to set up on the cluster") } - pullSecretYAML := d.dockerAuth.CreatePullSecretYAMLFromCredentials(*d.dockerCreds, namespace) + pullSecretYAML := d.dockerAuth.CreatePullSecretYAMLFromCredentials(*d.dockerCreds, namespace, d.config.Roxie.Registry()) _, err := d.runKubectl(ctx, k8s.KubectlOptions{ Args: []string{"apply", "-f", "-"}, Stdin: strings.NewReader(pullSecretYAML), diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index f399a3cb..a6a6d308 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -358,7 +358,7 @@ func (d *Deployer) prepareCredentials() error { d.logger.Dimf("Preparing and verifying Docker credentials...") // This will retrieve and verify credentials, returning error if invalid - creds, err := d.dockerAuth.GetAndVerifyCredentials() + creds, err := d.dockerAuth.GetAndVerifyCredentials(d.config.Roxie.Registry()) if err != nil { return err } diff --git a/internal/deployer/konflux_test.go b/internal/deployer/konflux_test.go index 08132e3a..62556346 100644 --- a/internal/deployer/konflux_test.go +++ b/internal/deployer/konflux_test.go @@ -12,13 +12,18 @@ import ( func TestOperatorImage_Konflux(t *testing.T) { instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(true)} expected := fmt.Sprintf("%s/release-operator:4.9.2", constants.DefaultRegistry) - assert.Equal(t, expected, instance.OperatorImage()) + assert.Equal(t, expected, instance.OperatorImage(constants.DefaultRegistry)) } func TestOperatorImage_NonKonflux(t *testing.T) { instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(false)} expected := fmt.Sprintf("%s/stackrox-operator:4.9.2", constants.DefaultRegistry) - assert.Equal(t, expected, instance.OperatorImage()) + assert.Equal(t, expected, instance.OperatorImage(constants.DefaultRegistry)) +} + +func TestOperatorImage_RegistryOverride(t *testing.T) { + instance := OperatorInstanceConfig{Version: "4.9.2"} + assert.Equal(t, "quay.io/stackrox-io/stackrox-operator:4.9.2", instance.OperatorImage("quay.io/stackrox-io")) } func TestPopulateKonfluxEnvVars_AllEntries(t *testing.T) { diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 20f5ed02..89187dd6 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -5,14 +5,17 @@ import ( "context" "errors" "fmt" + "net/http" "os" "path/filepath" "strings" "time" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" "gopkg.in/yaml.v3" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/k8s" "github.com/stackrox/roxie/internal/ocihelper" ) @@ -35,7 +38,10 @@ var requiredCRDs = []string{ // deployOperatorNonOLM deploys one RHACS operator instance without OLM. func (d *Deployer) deployOperatorNonOLM(ctx context.Context, instance OperatorInstanceConfig) error { d.logger.Infof("Operator tag: %s (namespace %s)", instance.Version, instance.Namespace) - bundleImage := instance.BundleImage() + bundleImage, err := d.resolveBundleImage(ctx, instance, d.config.Roxie.Registry()) + if err != nil { + return fmt.Errorf("resolving operator bundle image: %w", err) + } bundleDir, err := d.downloadAndExtractOperatorBundle(ctx, bundleImage) if err != nil { @@ -168,7 +174,10 @@ func (d *Deployer) ensureCRDsInstalled(ctx context.Context) error { if len(missing) > 0 { crdInstance := d.config.NewestOperatorInstance() - bundleImage := crdInstance.BundleImage() + bundleImage, err := d.resolveBundleImage(ctx, crdInstance, d.config.Roxie.Registry()) + if err != nil { + return fmt.Errorf("resolving operator bundle image: %w", err) + } d.logger.Warningf("Missing CRDs detected (%s)", strings.Join(missing, ", ")) d.logger.Warningf("Fetching bundle %s", bundleImage) @@ -189,6 +198,30 @@ func (d *Deployer) ensureCRDsInstalled(ctx context.Context) error { return nil } +// resolveBundleImage returns the operator bundle image to use for the given instance, probing +// the configured registry first and falling back to constants.DefaultRegistry if the bundle +// does not exist there. +// +// This is done because upstream StackRox builds (quay.io/stackrox-io) do not publish operator bundles. +func (d *Deployer) resolveBundleImage(ctx context.Context, instance OperatorInstanceConfig, registry string) (string, error) { + bundleImage := instance.BundleImage(registry) + if registry == constants.DefaultRegistry { + return bundleImage, nil + } + + if err := ocihelper.VerifyImageExistence(ctx, d.logger, bundleImage); err != nil { + var te *transport.Error + if errors.As(err, &te) && te.StatusCode == http.StatusNotFound { + fallbackImage := instance.BundleImage(constants.DefaultRegistry) + d.logger.Infof("No operator bundle found at %s, falling back to %s", bundleImage, fallbackImage) + return fallbackImage, nil + } + return "", fmt.Errorf("verifying operator bundle %s: %w", bundleImage, err) + } + + return bundleImage, nil +} + // deployOperatorFromCSV deploys the operator from CSV into the given instance namespace. func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string, instance OperatorInstanceConfig) error { csvFile := filepath.Join(bundleDir, "rhacs-operator.clusterserviceversion.yaml") @@ -440,11 +473,11 @@ func (d *Deployer) createDeploymentFromCSV(ctx context.Context, instance Operato return fmt.Errorf("extracting manager container from operator pod spec: %w", err) } + operatorImage := instance.OperatorImage(d.config.Roxie.Registry()) podSpec["serviceAccountName"] = deploymentSpec["service_account"] - if current, _ := managerContainer["image"].(string); current != instance.OperatorImage() { - // Currently this should only happen in Konflux mode. - d.logger.Infof("Rewriting operator image to %s", instance.OperatorImage()) - managerContainer["image"] = instance.OperatorImage() + if current, _ := managerContainer["image"].(string); current != operatorImage { + d.logger.Infof("Rewriting operator image to %s", operatorImage) + managerContainer["image"] = operatorImage } if len(instance.EnvVars) > 0 { diff --git a/internal/deployer/operator_integration_test.go b/internal/deployer/operator_integration_test.go new file mode 100644 index 00000000..f594d0c6 --- /dev/null +++ b/internal/deployer/operator_integration_test.go @@ -0,0 +1,39 @@ +//go:build integration + +package deployer + +import ( + "context" + "testing" + "time" + + "github.com/stackrox/roxie/internal/constants" + "github.com/stackrox/roxie/internal/logger" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveBundleImage_StackroxIOFallsBackToDefault_Integration(t *testing.T) { + d := &Deployer{logger: logger.New()} + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Minute) + defer cancel() + + instance := OperatorInstanceConfig{Version: "4.11.1"} + + // We don't build operator bundles for upstream StackRox builds, so this should fall back to the rhacs-eng-hosted bundle. + bundleImage, err := d.resolveBundleImage(ctx, instance, "quay.io/stackrox-io") + require.NoError(t, err) + assert.Equal(t, constants.DefaultRegistry+"/stackrox-operator-bundle:v4.11.1", bundleImage, + "should fall back to the rhacs-eng-hosted bundle") +} + +func TestResolveBundleImage_NonNotFoundErrorPropagates_Integration(t *testing.T) { + d := &Deployer{logger: logger.New()} + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + + instance := OperatorInstanceConfig{Version: "4.11.1"} + + _, err := d.resolveBundleImage(ctx, instance, "roxie-test-nonexistent-host.invalid/rhacs-eng") + require.Error(t, err) +} diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index fde6bd07..3371bfd7 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -9,15 +9,18 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/logger" ) -const ( - acsImageRegistry = "quay.io" - mainImageRepository = "rhacs-eng/main" -) +// splitRegistryHost splits a resolved image registry (e.g. "quay.io/stackrox-io") +// into its host ("quay.io") and org/repo path ("stackrox-io"). +func splitRegistryHost(registry string) (host, path string) { + host, path, _ = strings.Cut(registry, "/") + return host, path +} // DockerAuth handles Docker authentication and pull secret management. type DockerAuth struct { @@ -58,7 +61,10 @@ func New(log *logger.Logger) *DockerAuth { // GetAndVerifyCredentials retrieves and verifies Docker credentials. // This should be called early to fail fast if credentials are invalid. -func (d *DockerAuth) GetAndVerifyCredentials() (*Credentials, error) { +func (d *DockerAuth) GetAndVerifyCredentials(registry string) (*Credentials, error) { + host, orgPath := splitRegistryHost(registry) + mainImageRepository := orgPath + "/main" + var username, password string // Try environment variables first. @@ -78,7 +84,7 @@ func (d *DockerAuth) GetAndVerifyCredentials() (*Credentials, error) { d.logger.Dimf("REGISTRY_USERNAME/REGISTRY_PASSWORD unset. Trying to obtain Docker credentials from config file: %s", dockerConfigPath) if _, err := os.Stat(dockerConfigPath); err == nil { var err error - username, password, err = d.getCredentialsFromDockerConfig(dockerConfigPath) + username, password, err = d.getCredentialsFromDockerConfig(dockerConfigPath, host) if err != nil { return nil, err } @@ -91,7 +97,7 @@ func (d *DockerAuth) GetAndVerifyCredentials() (*Credentials, error) { // Verify credentials. if !d.skipCredVerification { - if err := d.VerifyCredentials(username, password); err != nil { + if err := d.VerifyCredentials(username, password, host, mainImageRepository); err != nil { return nil, fmt.Errorf("credentials are invalid: %w", err) } } @@ -102,8 +108,9 @@ func (d *DockerAuth) GetAndVerifyCredentials() (*Credentials, error) { }, nil } -// getCredentialsFromDockerConfig extracts credentials from existing Docker config. -func (d *DockerAuth) getCredentialsFromDockerConfig(configPath string) (string, string, error) { +// getCredentialsFromDockerConfig extracts credentials from existing Docker config +// for the given registry host. +func (d *DockerAuth) getCredentialsFromDockerConfig(configPath, host string) (string, string, error) { data, err := os.ReadFile(configPath) if err != nil { return "", "", fmt.Errorf("failed to read Docker config: %w", err) @@ -114,8 +121,8 @@ func (d *DockerAuth) getCredentialsFromDockerConfig(configPath string) (string, return "", "", fmt.Errorf("failed to parse Docker config: %w", err) } - // Check for existing auths for the ACS image registry. - if authEntry, ok := config.Auths[acsImageRegistry]; ok && authEntry.Auth != "" { + // Check for existing auths for the target registry host. + if authEntry, ok := config.Auths[host]; ok && authEntry.Auth != "" { // Decode the base64 auth string to get username:password decoded, err := base64.StdEncoding.DecodeString(authEntry.Auth) if err != nil { @@ -128,15 +135,15 @@ func (d *DockerAuth) getCredentialsFromDockerConfig(configPath string) (string, return string(parts[0]), string(parts[1]), nil } - // Try credential helper specifically configured for the ACS image registry - helper := d.lookupCredentialHelperForRegistry(&config, acsImageRegistry) + // Try credential helper specifically configured for the target registry host. + helper := d.lookupCredentialHelperForRegistry(&config, host) if helper == "" { - return "", "", fmt.Errorf("no Docker credentials found in config for ACS image registry (%s)", acsImageRegistry) + return "", "", fmt.Errorf("no Docker credentials found in config for image registry (%s)", host) } - credData, err := d.getCredentialFromHelper(helper, acsImageRegistry) + credData, err := d.getCredentialFromHelper(helper, host) if err != nil { - return "", "", fmt.Errorf("failed to get credentials from helper '%s' for '%s': %w", helper, acsImageRegistry, err) + return "", "", fmt.Errorf("failed to get credentials from helper '%s' for '%s': %w", helper, host, err) } return credData.Username, credData.Secret, nil @@ -177,19 +184,19 @@ func (d *DockerAuth) getCredentialFromHelper(helperName, registry string) (*Cred return &credData, nil } -// VerifyCredentials attempts to verify that the credentials work by making a request to the registry. -// This uses a read-only HTTP request. +// VerifyCredentials attempts to verify that the credentials work by making a request to the +// given registry host for the given repository. This uses a read-only HTTP request. // It mimics what the kubelet would do when pulling images. -func (d *DockerAuth) VerifyCredentials(username, password string) error { +func (d *DockerAuth) VerifyCredentials(username, password, host, repository string) error { // Create auth header for Basic authentication authString := fmt.Sprintf("%s:%s", username, password) encodedAuth := base64.StdEncoding.EncodeToString([]byte(authString)) - // Try to get a token from quay.io's OAuth2 endpoint for a specific repository + // Try to get a token from the registry's OAuth2 endpoint for a specific repository // This mimics what kubelet does when pulling images - it requests a token with pull scope // for the specific repository. authURL := fmt.Sprintf("https://%s/v2/auth?service=%s&scope=repository:%s:pull", - acsImageRegistry, acsImageRegistry, mainImageRepository) + host, host, repository) cmd := exec.Command("curl", "-s", "-f", "-H", fmt.Sprintf("Authorization: Basic %s", encodedAuth), @@ -197,34 +204,37 @@ func (d *DockerAuth) VerifyCredentials(username, password string) error { output, err := cmd.CombinedOutput() if err != nil { - d.logger.Warningf("Failed to verify credentials for %s: %v", acsImageRegistry, err) + d.logger.Warningf("Failed to verify credentials for %s: %v", host, err) d.logger.Dimf("Verification output: %s", string(output)) - return fmt.Errorf("credential verification failed for %s: %w", acsImageRegistry, err) + return fmt.Errorf("credential verification failed for %s: %w", host, err) } // Check if we got a valid JSON response with a token var tokenResponse map[string]interface{} if err := json.Unmarshal(output, &tokenResponse); err != nil { - return fmt.Errorf("credential verification failed: invalid response from %s: %w", acsImageRegistry, err) + return fmt.Errorf("credential verification failed: invalid response from %s: %w", host, err) } if _, ok := tokenResponse["token"]; !ok { - return fmt.Errorf("credential verification failed: no token received from %s", acsImageRegistry) + return fmt.Errorf("credential verification failed: no token received from %s", host) } - d.logger.Dimf("Successfully verified credentials for %s (repository: %s)", acsImageRegistry, mainImageRepository) + d.logger.Dimf("Successfully verified credentials for %s (repository: %s)", host, repository) return nil } -// CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from verified credentials. -func (d *DockerAuth) CreatePullSecretYAMLFromCredentials(creds Credentials, namespace string) string { +// CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from +// verified credentials, scoped to the host of the given image registry +func (d *DockerAuth) CreatePullSecretYAMLFromCredentials(creds Credentials, namespace, registry string) string { + host, _ := splitRegistryHost(registry) + // Create auth string authString := fmt.Sprintf("%s:%s", creds.Username, creds.Password) encodedAuth := base64.StdEncoding.EncodeToString([]byte(authString)) dockerConfig := DockerConfig{ Auths: map[string]AuthEntry{ - acsImageRegistry: {Auth: encodedAuth}, + host: {Auth: encodedAuth}, }, } diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index 597acc23..244ac574 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/logger" ) @@ -18,7 +19,7 @@ func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { da := New(log) da.skipCredVerification = true // Skip verification in tests - creds, err := da.GetAndVerifyCredentials() + creds, err := da.GetAndVerifyCredentials(constants.DefaultRegistry) if err != nil { t.Fatalf("GetAndVerifyCredentials failed: %v", err) } @@ -31,7 +32,7 @@ func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { } // Test creating YAML from credentials - yamlText := da.CreatePullSecretYAMLFromCredentials(*creds, "ns") + yamlText := da.CreatePullSecretYAMLFromCredentials(*creds, "ns", "registry.example.com/some-org") // Verify YAML structure if !strings.Contains(yamlText, "apiVersion: v1") { @@ -72,8 +73,12 @@ func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { t.Fatalf("Decoded data is not valid JSON: %v", err) } - if _, ok := data["auths"]; !ok { - t.Error("Decoded JSON should contain 'auths' key") + auths, ok := data["auths"].(map[string]interface{}) + if !ok { + t.Fatal("Decoded JSON should contain 'auths' key") + } + if _, ok := auths["registry.example.com"]; !ok { + t.Errorf("Expected auths to be keyed by the registry host 'registry.example.com', got %v", auths) } } @@ -89,8 +94,33 @@ func TestGetAndVerifyCredentialsNoCredentials(t *testing.T) { da := New(log) da.skipCredVerification = true // Skip verification in tests - _, err := da.GetAndVerifyCredentials() + _, err := da.GetAndVerifyCredentials(constants.DefaultRegistry) if err == nil { t.Error("Expected error when no credentials are available") } } +func TestSplitRegistryHost(t *testing.T) { + tests := []struct { + name string + registry string + expectedHost string + expectedPath string + }{ + {"default registry", constants.DefaultRegistry, "quay.io", "rhacs-eng"}, + {"quay.io with org", "quay.io/stackrox-io", "quay.io", "stackrox-io"}, + {"registry with port and nested path", "registry.io:5000/org/suborg", "registry.io:5000", "org/suborg"}, + {"just hostname", "justahost", "justahost", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + host, path := splitRegistryHost(tt.registry) + if host != tt.expectedHost { + t.Errorf("splitRegistryHost(%q): expected host %q, got %q", tt.registry, tt.expectedHost, host) + } + if path != tt.expectedPath { + t.Errorf("splitRegistryHost(%q): expected path %q, got %q", tt.registry, tt.expectedPath, path) + } + }) + } +} diff --git a/tests/e2e/custom_registry_test.go b/tests/e2e/custom_registry_test.go new file mode 100644 index 00000000..52ff0c0e --- /dev/null +++ b/tests/e2e/custom_registry_test.go @@ -0,0 +1,59 @@ +//go:build e2e + +package e2e + +import ( + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestDeployWithStackroxIORegistry verifies that roxie can deploy Central using +// the public quay.io/stackrox-io registry instead of the default quay.io/rhacs-eng. +func TestDeployWithStackroxIORegistry(t *testing.T) { + dumpClusterStateOnFailure(t) + + const stackroxIORegistry = "quay.io/stackrox-io" + + envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") + require.NoError(t, err) + envrcPath := envrcFile.Name() + envrcFile.Close() + + t.Log("=== Deploying central with quay.io/stackrox-io registry ===") + args := append([]string{ + roxieBinary, "deploy", "--early-readiness", "central", + "--set", "roxie.imageRegistry=" + stackroxIORegistry, + "--envrc", envrcPath, + }, commonDeployArgs...) + runCommand(t, deployTimeout, nil, args...) + + verifyCentralInstalled(t, centralNamespace) + verifyOperatorDeploymentExists(t, operatorSystemNamespace) + verifyOperatorImageRegistry(t, operatorSystemNamespace, stackroxIORegistry) + + t.Log("=== Cleaning up ===") + teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"} + runCommand(t, teardownTimeout, nil, teardownArgs...) + + verifyCentralNotInstalled(t, centralNamespace) +} + +// verifyOperatorImageRegistry asserts that the operator deployment's image is +// hosted on the expected registry. +func verifyOperatorImageRegistry(t *testing.T, namespace, expectedRegistry string) { + t.Helper() + + cmd := exec.Command("kubectl", "get", "deployment", operatorDeploymentName, "-n", namespace, + "-o", "jsonpath={.spec.template.spec.containers[0].image}") + output, err := cmd.Output() + require.NoErrorf(t, err, "Failed to get operator image in namespace %s", namespace) + + image := strings.TrimSpace(string(output)) + require.Truef(t, strings.HasPrefix(image, expectedRegistry+"/"), + "Expected operator image to be pulled from %s, got: %s", expectedRegistry, image) + t.Logf("✓ Operator image %s uses registry %s", image, expectedRegistry) +} From 1724e1d703650693861358c832e36190163f915f Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Thu, 13 Aug 2026 14:20:00 +0200 Subject: [PATCH 02/14] Make VerifyCredentials work with registries other than quay.io --- internal/deployer/deployer.go | 6 ++-- internal/dockerauth/dockerauth.go | 49 ++++++++++---------------- internal/dockerauth/dockerauth_test.go | 5 +-- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index a6a6d308..3b429604 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -305,7 +305,7 @@ func (d *Deployer) Deploy(ctx context.Context, components component.Component) e // Prepare and verify credentials early to fail fast. needPullSecrets := d.config.Roxie.ClusterType.NeedsPullSecrets() if needPullSecrets { - if err := d.prepareCredentials(); err != nil { + if err := d.prepareCredentials(ctx); err != nil { return fmt.Errorf("failed to prepare credentials: %w", err) } } @@ -354,11 +354,11 @@ func (d *Deployer) Deploy(ctx context.Context, components component.Component) e // prepareCredentials prepares and verifies Docker credentials early to allow failing fast. // The verified credentials are stored in the Deployer object for later use. -func (d *Deployer) prepareCredentials() error { +func (d *Deployer) prepareCredentials(ctx context.Context) error { d.logger.Dimf("Preparing and verifying Docker credentials...") // This will retrieve and verify credentials, returning error if invalid - creds, err := d.dockerAuth.GetAndVerifyCredentials(d.config.Roxie.Registry()) + creds, err := d.dockerAuth.GetAndVerifyCredentials(ctx, d.config.Roxie.Registry()) if err != nil { return err } diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index 3371bfd7..491158e0 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -2,15 +2,21 @@ package dockerauth import ( "bytes" + "context" "encoding/base64" "encoding/json" "errors" "fmt" + "net/http" "os" "os/exec" "path/filepath" "strings" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/logger" ) @@ -61,7 +67,7 @@ func New(log *logger.Logger) *DockerAuth { // GetAndVerifyCredentials retrieves and verifies Docker credentials. // This should be called early to fail fast if credentials are invalid. -func (d *DockerAuth) GetAndVerifyCredentials(registry string) (*Credentials, error) { +func (d *DockerAuth) GetAndVerifyCredentials(ctx context.Context, registry string) (*Credentials, error) { host, orgPath := splitRegistryHost(registry) mainImageRepository := orgPath + "/main" @@ -97,7 +103,7 @@ func (d *DockerAuth) GetAndVerifyCredentials(registry string) (*Credentials, err // Verify credentials. if !d.skipCredVerification { - if err := d.VerifyCredentials(username, password, host, mainImageRepository); err != nil { + if err := d.VerifyCredentials(ctx, username, password, host, mainImageRepository); err != nil { return nil, fmt.Errorf("credentials are invalid: %w", err) } } @@ -184,39 +190,20 @@ func (d *DockerAuth) getCredentialFromHelper(helperName, registry string) (*Cred return &credData, nil } -// VerifyCredentials attempts to verify that the credentials work by making a request to the -// given registry host for the given repository. This uses a read-only HTTP request. -// It mimics what the kubelet would do when pulling images. -func (d *DockerAuth) VerifyCredentials(username, password, host, repository string) error { - // Create auth header for Basic authentication - authString := fmt.Sprintf("%s:%s", username, password) - encodedAuth := base64.StdEncoding.EncodeToString([]byte(authString)) - - // Try to get a token from the registry's OAuth2 endpoint for a specific repository - // This mimics what kubelet does when pulling images - it requests a token with pull scope - // for the specific repository. - authURL := fmt.Sprintf("https://%s/v2/auth?service=%s&scope=repository:%s:pull", - host, host, repository) - - cmd := exec.Command("curl", "-s", "-f", - "-H", fmt.Sprintf("Authorization: Basic %s", encodedAuth), - authURL) - - output, err := cmd.CombinedOutput() +// VerifyCredentials verifies that the given credentials grant pull access to +// the given repository on the given registry host. It works for registries +// that follow the standard OCI Distribution v2 challenge/token protocol. +func (d *DockerAuth) VerifyCredentials(ctx context.Context, username, password, host, repository string) error { + reg, err := name.NewRegistry(host) if err != nil { - d.logger.Warningf("Failed to verify credentials for %s: %v", host, err) - d.logger.Dimf("Verification output: %s", string(output)) - return fmt.Errorf("credential verification failed for %s: %w", host, err) + return fmt.Errorf("invalid registry host %q: %w", host, err) } - // Check if we got a valid JSON response with a token - var tokenResponse map[string]interface{} - if err := json.Unmarshal(output, &tokenResponse); err != nil { - return fmt.Errorf("credential verification failed: invalid response from %s: %w", host, err) - } + auth := &authn.Basic{Username: username, Password: password} + scope := fmt.Sprintf("repository:%s:pull", repository) - if _, ok := tokenResponse["token"]; !ok { - return fmt.Errorf("credential verification failed: no token received from %s", host) + if _, err := transport.NewWithContext(ctx, reg, auth, http.DefaultTransport, []string{scope}); err != nil { + return fmt.Errorf("credential verification failed for %s: %w", host, err) } d.logger.Dimf("Successfully verified credentials for %s (repository: %s)", host, repository) diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index 244ac574..b6b26517 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -1,6 +1,7 @@ package dockerauth import ( + "context" "encoding/base64" "encoding/json" "strings" @@ -19,7 +20,7 @@ func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { da := New(log) da.skipCredVerification = true // Skip verification in tests - creds, err := da.GetAndVerifyCredentials(constants.DefaultRegistry) + creds, err := da.GetAndVerifyCredentials(context.Background(), constants.DefaultRegistry) if err != nil { t.Fatalf("GetAndVerifyCredentials failed: %v", err) } @@ -94,7 +95,7 @@ func TestGetAndVerifyCredentialsNoCredentials(t *testing.T) { da := New(log) da.skipCredVerification = true // Skip verification in tests - _, err := da.GetAndVerifyCredentials(constants.DefaultRegistry) + _, err := da.GetAndVerifyCredentials(context.Background(), constants.DefaultRegistry) if err == nil { t.Error("Expected error when no credentials are available") } From 95928d5ba28b9c8a39b2174cec9bdb10e424c7bf Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Thu, 13 Aug 2026 20:31:12 +0200 Subject: [PATCH 03/14] Rewrite logic that determines if the cluster needs pull secrets --- cmd/deploy.go | 33 +++++--- internal/deployer/addons.go | 2 +- internal/deployer/config.go | 18 +++++ internal/deployer/deploy_via_operator.go | 4 +- internal/deployer/deployer.go | 2 +- internal/deployer/operator.go | 9 ++- internal/deployer/operator_test.go | 95 ++++++++++++++++++++++++ internal/dockerauth/dockerauth.go | 43 +++++++++++ internal/dockerauth/dockerauth_test.go | 89 ++++++++++++++++++++++ internal/types/cluster_type.go | 4 +- 10 files changed, 283 insertions(+), 16 deletions(-) create mode 100644 internal/deployer/operator_test.go diff --git a/cmd/deploy.go b/cmd/deploy.go index b15e6481..d479b308 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -18,6 +18,7 @@ import ( "github.com/stackrox/roxie/internal/component" "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/deployer" + "github.com/stackrox/roxie/internal/dockerauth" "github.com/stackrox/roxie/internal/env" "github.com/stackrox/roxie/internal/helpers" "github.com/stackrox/roxie/internal/imagetag" @@ -267,7 +268,7 @@ func runDeploy(cmd *cobra.Command, args []string) error { return err } - if err := deployValidate(log, components, &deploySettings); err != nil { + if err := deployValidate(ctx, log, components, &deploySettings); err != nil { return err } @@ -464,7 +465,7 @@ func validateImageRegistry(registry string) error { return nil } -func deployValidate(log *logger.Logger, components component.Component, deploySettings *deployer.Config) error { +func deployValidate(ctx context.Context, log *logger.Logger, components component.Component, deploySettings *deployer.Config) error { if components.IncludesCentral() && os.Getenv("ROXIE_SHELL") != "" { return errors.New("already in a roxie sub-shell (ROXIE_SHELL environment variable is set), please exit the shell and try again") } @@ -473,6 +474,24 @@ func deployValidate(log *logger.Logger, components component.Component, deploySe return errors.New("running without a controlling terminal requires --envrc to be set") } + registry := deploySettings.Roxie.Registry() + if deploySettings.Roxie.UsesCustomRegistry() { + if err := validateImageRegistry(registry); err != nil { + return err + } + + requiresAuth, err := dockerauth.New(log).RegistryRequiresAuth(ctx, registry) + if err != nil { + return fmt.Errorf("checking registry %s: %w", registry, err) + } + deploySettings.Roxie.RegistryRequiresAuth = requiresAuth + if requiresAuth { + log.Dimf("Registry %s requires authentication", registry) + } else { + log.Dimf("Registry %s is public, no authentication required", registry) + } + } + clusterType := deploySettings.Roxie.ClusterType if env.RunningInRoxieContainer { @@ -484,10 +503,9 @@ func deployValidate(log *logger.Logger, components component.Component, deploySe return errors.New("containerized mode requires Central exposure") } - // On infra OpenShift we already get image pull secrets for Quay automatically. - if clusterType.NeedsPullSecrets() { + if deploySettings.Roxie.NeedsPullSecrets() { if os.Getenv("REGISTRY_USERNAME") == "" || os.Getenv("REGISTRY_PASSWORD") == "" { - return fmt.Errorf("containerized mode requires REGISTRY_USERNAME and REGISTRY_PASSWORD environment variables for clusters of type %s", clusterType) + return fmt.Errorf("containerized mode requires REGISTRY_USERNAME and REGISTRY_PASSWORD environment variables for registry %s on clusters of type %s", registry, clusterType) } if _, err := os.Stat("/kubeconfig"); err != nil { return fmt.Errorf("containerized mode requires /kubeconfig file: %w", err) @@ -499,11 +517,6 @@ func deployValidate(log *logger.Logger, components component.Component, deploySe return errors.New("skipping operator deployment while also requesting deploying via OLM at the same time does not make sense") } - registry := deploySettings.Roxie.Registry() - if err := validateImageRegistry(registry); err != nil { - return err - } - if deploySettings.Roxie.KonfluxImagesEnabled() { if deploySettings.Operator.DeployViaOlmEnabled() { return errors.New("using Konflux images while deploying operator via OLM is not supported") diff --git a/internal/deployer/addons.go b/internal/deployer/addons.go index 3d683db6..2dd47f78 100644 --- a/internal/deployer/addons.go +++ b/internal/deployer/addons.go @@ -31,7 +31,7 @@ func (d *Deployer) deployAddOns(ctx context.Context, addOns []AddOn) error { return nil } - needPullSecrets := d.config.Roxie.ClusterType.NeedsPullSecrets() + needPullSecrets := d.config.Roxie.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.Central.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } diff --git a/internal/deployer/config.go b/internal/deployer/config.go index 988fd2ff..e4c3e496 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -62,6 +62,9 @@ type RoxieConfig struct { FeatureFlags map[string]bool `yaml:"featureFlags,omitempty"` ClusterType types.ClusterType `yaml:"clusterType,omitempty"` HAProxy HAProxyConfig `yaml:"haProxy,omitempty"` + + // RegistryRequiresAuth is computed internally and is not user-configurable. + RegistryRequiresAuth bool `yaml:"-"` } // Registry returns the resolved image registry, defaulting to @@ -73,6 +76,21 @@ func (c *RoxieConfig) Registry() string { return strings.TrimSuffix(c.ImageRegistry, "/") } +// UsesCustomRegistry returns whether a custom image registry was configured. +func (c *RoxieConfig) UsesCustomRegistry() bool { + return c.Registry() != constants.DefaultRegistry +} + +// NeedsPullSecrets returns whether roxie needs to set up image pull secrets itself. +// For a custom registry this relies on RegistryRequiresAuth having already been +// resolved during deploy validation (see cmd/deploy.go's deployValidate). +func (c *RoxieConfig) NeedsPullSecrets() bool { + if c.UsesCustomRegistry() { + return c.RegistryRequiresAuth + } + return c.ClusterType.NeedsDefaultRegistryPullSecrets() +} + func (c *RoxieConfig) KonfluxImagesSet() bool { return c.KonfluxImages != nil } diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 323329d2..1ac97859 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -215,7 +215,7 @@ func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { func (d *Deployer) deployCentralOperator(ctx context.Context) error { d.logger.Info("🚀 Deploying Central via Operator...") - needPullSecrets := d.config.Roxie.ClusterType.NeedsPullSecrets() + needPullSecrets := d.config.Roxie.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.Central.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } @@ -822,7 +822,7 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context) error { func (d *Deployer) deploySecuredClusterOperator(ctx context.Context) error { d.logger.Info("🚀 Deploying SecuredCluster via Operator...") - needPullSecrets := d.config.Roxie.ClusterType.NeedsPullSecrets() + needPullSecrets := d.config.Roxie.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.SecuredCluster.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index 3b429604..b829f719 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -303,7 +303,7 @@ func (d *Deployer) stopDetachedPortForward() { // Deploy deploys the specified components to the cluster. func (d *Deployer) Deploy(ctx context.Context, components component.Component) error { // Prepare and verify credentials early to fail fast. - needPullSecrets := d.config.Roxie.ClusterType.NeedsPullSecrets() + needPullSecrets := d.config.Roxie.NeedsPullSecrets() if needPullSecrets { if err := d.prepareCredentials(ctx); err != nil { return fmt.Errorf("failed to prepare credentials: %w", err) diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 89187dd6..8f563c5f 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -222,6 +222,13 @@ func (d *Deployer) resolveBundleImage(ctx context.Context, instance OperatorInst return bundleImage, nil } +func needsOperatorPullSecrets(instance OperatorInstanceConfig, roxieConfig *RoxieConfig) bool { + if roxieConfig.UsesCustomRegistry() { + return roxieConfig.RegistryRequiresAuth + } + return instance.KonfluxImagesEnabled() && roxieConfig.ClusterType.NeedsDefaultRegistryPullSecrets() +} + // deployOperatorFromCSV deploys the operator from CSV into the given instance namespace. func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string, instance OperatorInstanceConfig) error { csvFile := filepath.Join(bundleDir, "rhacs-operator.clusterserviceversion.yaml") @@ -237,7 +244,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string, } serviceAccountName := deploymentSpec["service_account"].(string) - d.useOperatorPullSecrets = instance.KonfluxImagesEnabled() && d.config.Roxie.ClusterType.NeedsPullSecrets() + d.useOperatorPullSecrets = needsOperatorPullSecrets(instance, &d.config.Roxie) d.logger.Info("📋 Operator deployment plan:") d.logger.Dimf(" • Namespace: %s", instance.Namespace) diff --git a/internal/deployer/operator_test.go b/internal/deployer/operator_test.go new file mode 100644 index 00000000..12a59902 --- /dev/null +++ b/internal/deployer/operator_test.go @@ -0,0 +1,95 @@ +package deployer + +import ( + "testing" + + "github.com/stackrox/roxie/internal/types" + "github.com/stretchr/testify/assert" +) + +func TestNeedsOperatorPullSecrets(t *testing.T) { + tests := []struct { + name string + instance OperatorInstanceConfig + roxieConfig RoxieConfig + expected bool + }{ + { + name: "default registry, non-Konflux: no pull secrets", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + expected: false, + }, + { + name: "Konflux images: pull secrets needed", + instance: OperatorInstanceConfig{KonfluxImages: new(true)}, + roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + expected: true, + }, + { + name: "Konflux images on a cluster type that auto-configures default-registry credentials: no pull secrets", + instance: OperatorInstanceConfig{KonfluxImages: new(true)}, + roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeInfraOpenShift4}, + expected: false, + }, + { + name: "private custom registry, non-Konflux: pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE, RegistryRequiresAuth: true}, + expected: true, + }, + { + name: "private custom registry is never auto-configured, even on a cluster type that auto-configures the default registry", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: true}, + expected: true, + }, + { + name: "public custom registry: no pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE, RegistryRequiresAuth: false}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, needsOperatorPullSecrets(tt.instance, &tt.roxieConfig)) + }) + } +} + +func TestRoxieConfig_NeedsPullSecrets(t *testing.T) { + tests := []struct { + name string + roxie RoxieConfig + expected bool + }{ + { + name: "default registry on a cluster type that auto-configures credentials", + roxie: RoxieConfig{ClusterType: types.ClusterTypeInfraOpenShift4}, + expected: false, + }, + { + name: "default registry on a cluster type that doesn't auto-configure credentials", + roxie: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + expected: true, + }, + { + name: "private custom registry, even on a cluster type that auto-configures default-registry credentials", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: true}, + expected: true, + }, + { + name: "public custom registry: no pull secrets needed", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: false}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.roxie.NeedsPullSecrets()) + }) + } +} diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index 491158e0..a64bf18b 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -210,6 +210,49 @@ func (d *DockerAuth) VerifyCredentials(ctx context.Context, username, password, return nil } +// RegistryRequiresAuth reports whether registry requires authentication to pull +// images. It makes a single, anonymous tags-list request against a well-known +// repository path and checks whether it's rejected. +func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) (bool, error) { + host, orgPath := splitRegistryHost(registry) + + reg, err := name.NewRegistry(host) + if err != nil { + return false, fmt.Errorf("invalid registry host %q: %w", host, err) + } + repo := reg.Repo(orgPath, "main") + + tr, err := transport.NewWithContext(ctx, reg, authn.Anonymous, http.DefaultTransport, []string{repo.Scope("pull")}) + if err != nil { + var te *transport.Error + if errors.As(err, &te) && indicatesAuthRequired(te.StatusCode) { + return true, nil + } + return false, fmt.Errorf("negotiating anonymous access to %s: %w", host, err) + } + + url := fmt.Sprintf("https://%s/v2/%s/tags/list?n=1", repo.RegistryStr(), repo.RepositoryStr()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return false, fmt.Errorf("building request for %s: %w", host, err) + } + + resp, err := (&http.Client{Transport: tr}).Do(req) + if err != nil { + return false, fmt.Errorf("checking whether %s requires authentication: %w", host, err) + } + defer resp.Body.Close() + + return indicatesAuthRequired(resp.StatusCode), nil +} + +// indicatesAuthRequired reports whether code suggests the anonymous request was +// rejected for lack of authentication. Besides 401/403, this also treats 404 as +// such, since some registries hide private repos behind it instead. +func indicatesAuthRequired(code int) bool { + return code == http.StatusUnauthorized || code == http.StatusForbidden || code == http.StatusNotFound +} + // CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from // verified credentials, scoped to the host of the given image registry func (d *DockerAuth) CreatePullSecretYAMLFromCredentials(creds Credentials, namespace, registry string) string { diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index b6b26517..200e901d 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -4,11 +4,16 @@ import ( "context" "encoding/base64" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" "strings" "testing" "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/logger" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { @@ -100,6 +105,90 @@ func TestGetAndVerifyCredentialsNoCredentials(t *testing.T) { t.Error("Expected error when no credentials are available") } } + +func TestRegistryRequiresAuth(t *testing.T) { + tests := []struct { + name string + challengeAuth bool // whether /v2/ demands a Bearer challenge at all + tokenStatus int // status the token endpoint returns, if challenged + tagsListStatus int // status the tags-list request returns + expectedRequires bool + }{ + { + name: "no auth mechanism: public", + challengeAuth: false, + tagsListStatus: http.StatusOK, + expectedRequires: false, + }, + { + name: "anonymous token granted, public repository", + challengeAuth: true, + tokenStatus: http.StatusOK, + tagsListStatus: http.StatusOK, + expectedRequires: false, + }, + { + name: "anonymous token granted, private repository", + challengeAuth: true, + tokenStatus: http.StatusOK, + tagsListStatus: http.StatusUnauthorized, + expectedRequires: true, + }, + { + name: "anonymous token granted, private repository hidden behind 404", + // Some registries (e.g. GHCR) return 404 instead of 401/403 for private + // repositories, to avoid leaking their existence to unauthenticated + // callers. + challengeAuth: true, + tokenStatus: http.StatusOK, + tagsListStatus: http.StatusNotFound, + expectedRequires: true, + }, + { + name: "anonymous token request itself rejected", + challengeAuth: true, + tokenStatus: http.StatusUnauthorized, + expectedRequires: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var registryAddr string + + mux := http.NewServeMux() + mux.HandleFunc("/v2/", func(w http.ResponseWriter, r *http.Request) { + if !tt.challengeAuth { + w.WriteHeader(http.StatusOK) + return + } + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer realm="http://%s/token",service="test-registry"`, registryAddr)) + w.WriteHeader(http.StatusUnauthorized) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + if tt.tokenStatus != http.StatusOK { + w.WriteHeader(tt.tokenStatus) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"token":"fake-anonymous-token"}`)) + }) + mux.HandleFunc("/v2/some-org/main/tags/list", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.tagsListStatus) + }) + + server := httptest.NewServer(mux) + defer server.Close() + registryAddr = strings.TrimPrefix(server.URL, "http://") + + da := &DockerAuth{logger: logger.New()} + requiresAuth, err := da.RegistryRequiresAuth(context.Background(), registryAddr+"/some-org") + require.NoError(t, err) + assert.Equal(t, tt.expectedRequires, requiresAuth) + }) + } +} + func TestSplitRegistryHost(t *testing.T) { tests := []struct { name string diff --git a/internal/types/cluster_type.go b/internal/types/cluster_type.go index fce6b8f8..de8d71bd 100644 --- a/internal/types/cluster_type.go +++ b/internal/types/cluster_type.go @@ -77,7 +77,9 @@ func (ct *ClusterType) UnmarshalYAML(unmarshal func(any) error) error { return fmt.Errorf("unknown cluster type identifier: %q", s) } -func (ct ClusterType) NeedsPullSecrets() bool { +// NeedsDefaultRegistryPullSecrets reports whether this cluster type lacks +// auto-configured credentials for the default image registry (quay.io/rhacs-eng). +func (ct ClusterType) NeedsDefaultRegistryPullSecrets() bool { return ct != ClusterTypeInfraOpenShift4 } From 2023295cb4154f7528046acd85dd4c81ac0bf39b Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Thu, 13 Aug 2026 22:09:48 +0200 Subject: [PATCH 04/14] simplify RegistryRequiresAuth to a best-effort bool check --- cmd/deploy.go | 5 +---- internal/dockerauth/dockerauth.go | 30 +++++++++----------------- internal/dockerauth/dockerauth_test.go | 16 ++++++++++---- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index d479b308..df625b90 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -480,10 +480,7 @@ func deployValidate(ctx context.Context, log *logger.Logger, components componen return err } - requiresAuth, err := dockerauth.New(log).RegistryRequiresAuth(ctx, registry) - if err != nil { - return fmt.Errorf("checking registry %s: %w", registry, err) - } + requiresAuth := dockerauth.New(log).RegistryRequiresAuth(ctx, registry) deploySettings.Roxie.RegistryRequiresAuth = requiresAuth if requiresAuth { log.Dimf("Registry %s requires authentication", registry) diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index a64bf18b..7d202f98 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -210,47 +210,37 @@ func (d *DockerAuth) VerifyCredentials(ctx context.Context, username, password, return nil } -// RegistryRequiresAuth reports whether registry requires authentication to pull -// images. It makes a single, anonymous tags-list request against a well-known -// repository path and checks whether it's rejected. -func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) (bool, error) { +// RegistryRequiresAuth makes a best-effort check for whether registry requires +// authentication to pull images, by sending a single anonymous tags-list +// request against a well-known repository path. Anything short of a confirmed +// successful response fails safe by reporting that auth is required. +func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) bool { host, orgPath := splitRegistryHost(registry) reg, err := name.NewRegistry(host) if err != nil { - return false, fmt.Errorf("invalid registry host %q: %w", host, err) + return true } repo := reg.Repo(orgPath, "main") tr, err := transport.NewWithContext(ctx, reg, authn.Anonymous, http.DefaultTransport, []string{repo.Scope("pull")}) if err != nil { - var te *transport.Error - if errors.As(err, &te) && indicatesAuthRequired(te.StatusCode) { - return true, nil - } - return false, fmt.Errorf("negotiating anonymous access to %s: %w", host, err) + return true } url := fmt.Sprintf("https://%s/v2/%s/tags/list?n=1", repo.RegistryStr(), repo.RepositoryStr()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { - return false, fmt.Errorf("building request for %s: %w", host, err) + return true } resp, err := (&http.Client{Transport: tr}).Do(req) if err != nil { - return false, fmt.Errorf("checking whether %s requires authentication: %w", host, err) + return true } defer resp.Body.Close() - return indicatesAuthRequired(resp.StatusCode), nil -} - -// indicatesAuthRequired reports whether code suggests the anonymous request was -// rejected for lack of authentication. Besides 401/403, this also treats 404 as -// such, since some registries hide private repos behind it instead. -func indicatesAuthRequired(code int) bool { - return code == http.StatusUnauthorized || code == http.StatusForbidden || code == http.StatusNotFound + return resp.StatusCode < 200 || resp.StatusCode >= 300 } // CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index 200e901d..f9c21ea3 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -13,7 +13,6 @@ import ( "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/logger" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestGetAndVerifyCredentialsFromEnv(t *testing.T) { @@ -136,7 +135,7 @@ func TestRegistryRequiresAuth(t *testing.T) { }, { name: "anonymous token granted, private repository hidden behind 404", - // Some registries (e.g. GHCR) return 404 instead of 401/403 for private + // Some registries return 404 instead of 401/403 for private // repositories, to avoid leaking their existence to unauthenticated // callers. challengeAuth: true, @@ -150,6 +149,16 @@ func TestRegistryRequiresAuth(t *testing.T) { tokenStatus: http.StatusUnauthorized, expectedRequires: true, }, + { + name: "tags-list request fails with a server error", + // A transient 5xx doesn't tell us whether the registry is public or + // private, so this fails safe by reporting that auth is required, + // rather than treating it as confirmed "public". + challengeAuth: true, + tokenStatus: http.StatusOK, + tagsListStatus: http.StatusInternalServerError, + expectedRequires: true, + }, } for _, tt := range tests { @@ -182,8 +191,7 @@ func TestRegistryRequiresAuth(t *testing.T) { registryAddr = strings.TrimPrefix(server.URL, "http://") da := &DockerAuth{logger: logger.New()} - requiresAuth, err := da.RegistryRequiresAuth(context.Background(), registryAddr+"/some-org") - require.NoError(t, err) + requiresAuth := da.RegistryRequiresAuth(context.Background(), registryAddr+"/some-org") assert.Equal(t, tt.expectedRequires, requiresAuth) }) } From 47fc50c0457b7bca7128b655a9c3c2feb22ba243 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Thu, 13 Aug 2026 22:28:41 +0200 Subject: [PATCH 05/14] don't hardcode https scheme in auth probe URL --- internal/dockerauth/dockerauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index 7d202f98..ac7c7863 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -228,7 +228,7 @@ func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) return true } - url := fmt.Sprintf("https://%s/v2/%s/tags/list?n=1", repo.RegistryStr(), repo.RepositoryStr()) + url := fmt.Sprintf("%s://%s/v2/%s/tags/list?n=1", repo.Scheme(), repo.RegistryStr(), repo.RepositoryStr()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return true From c91e9d5d7763b1d4ca83631c4383c76e61a343c7 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Sun, 16 Aug 2026 21:26:46 +0200 Subject: [PATCH 06/14] Move RegistryRequiresAuth and NeedsPullSecrets to Deployer --- cmd/deploy.go | 45 ++++++++-------- internal/deployer/addons.go | 2 +- internal/deployer/config.go | 13 ----- internal/deployer/deploy_via_operator.go | 4 +- internal/deployer/deployer.go | 29 ++++++++++- internal/deployer/operator.go | 12 +++-- internal/deployer/operator_test.go | 65 ++++++++++++++---------- 7 files changed, 98 insertions(+), 72 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index df625b90..497ed728 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -18,7 +18,6 @@ import ( "github.com/stackrox/roxie/internal/component" "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/deployer" - "github.com/stackrox/roxie/internal/dockerauth" "github.com/stackrox/roxie/internal/env" "github.com/stackrox/roxie/internal/helpers" "github.com/stackrox/roxie/internal/imagetag" @@ -268,7 +267,7 @@ func runDeploy(cmd *cobra.Command, args []string) error { return err } - if err := deployValidate(ctx, log, components, &deploySettings); err != nil { + if err := deployValidate(log, components, &deploySettings); err != nil { return err } @@ -284,6 +283,14 @@ func runDeploy(cmd *cobra.Command, args []string) error { d.SetVerbose(verbose) d.SetConfig(deploySettings) + d.ResolveRegistryAuth(ctx) + + if d.NeedsPullSecrets() { + if err := validateContainerizedCredentials(deploySettings.Roxie.Registry(), deploySettings.Roxie.ClusterType); err != nil { + return err + } + } + if dryRun { log.Info("Exiting because of enabled dry run mode.") return nil @@ -465,7 +472,20 @@ func validateImageRegistry(registry string) error { return nil } -func deployValidate(ctx context.Context, log *logger.Logger, components component.Component, deploySettings *deployer.Config) error { +func validateContainerizedCredentials(registry string, clusterType types.ClusterType) error { + if !env.RunningInRoxieContainer { + return nil + } + if os.Getenv("REGISTRY_USERNAME") == "" || os.Getenv("REGISTRY_PASSWORD") == "" { + return fmt.Errorf("containerized mode requires REGISTRY_USERNAME and REGISTRY_PASSWORD environment variables for registry %s on clusters of type %s", registry, clusterType) + } + if _, err := os.Stat("/kubeconfig"); err != nil { + return fmt.Errorf("containerized mode requires /kubeconfig file: %w", err) + } + return nil +} + +func deployValidate(log *logger.Logger, components component.Component, deploySettings *deployer.Config) error { if components.IncludesCentral() && os.Getenv("ROXIE_SHELL") != "" { return errors.New("already in a roxie sub-shell (ROXIE_SHELL environment variable is set), please exit the shell and try again") } @@ -479,18 +499,8 @@ func deployValidate(ctx context.Context, log *logger.Logger, components componen if err := validateImageRegistry(registry); err != nil { return err } - - requiresAuth := dockerauth.New(log).RegistryRequiresAuth(ctx, registry) - deploySettings.Roxie.RegistryRequiresAuth = requiresAuth - if requiresAuth { - log.Dimf("Registry %s requires authentication", registry) - } else { - log.Dimf("Registry %s is public, no authentication required", registry) - } } - clusterType := deploySettings.Roxie.ClusterType - if env.RunningInRoxieContainer { // For running containerized we have specific requirements. if deploySettings.Central.PortForwardingEnabled() { @@ -499,15 +509,6 @@ func deployValidate(ctx context.Context, log *logger.Logger, components componen if !deploySettings.Central.ExposureEnabled() { return errors.New("containerized mode requires Central exposure") } - - if deploySettings.Roxie.NeedsPullSecrets() { - if os.Getenv("REGISTRY_USERNAME") == "" || os.Getenv("REGISTRY_PASSWORD") == "" { - return fmt.Errorf("containerized mode requires REGISTRY_USERNAME and REGISTRY_PASSWORD environment variables for registry %s on clusters of type %s", registry, clusterType) - } - if _, err := os.Stat("/kubeconfig"); err != nil { - return fmt.Errorf("containerized mode requires /kubeconfig file: %w", err) - } - } } if deploySettings.Operator.SkipDeploymentEnabled() && deploySettings.Operator.DeployViaOlmEnabled() { diff --git a/internal/deployer/addons.go b/internal/deployer/addons.go index 2dd47f78..b9426b17 100644 --- a/internal/deployer/addons.go +++ b/internal/deployer/addons.go @@ -31,7 +31,7 @@ func (d *Deployer) deployAddOns(ctx context.Context, addOns []AddOn) error { return nil } - needPullSecrets := d.config.Roxie.NeedsPullSecrets() + needPullSecrets := d.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.Central.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } diff --git a/internal/deployer/config.go b/internal/deployer/config.go index e4c3e496..bc912416 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -62,9 +62,6 @@ type RoxieConfig struct { FeatureFlags map[string]bool `yaml:"featureFlags,omitempty"` ClusterType types.ClusterType `yaml:"clusterType,omitempty"` HAProxy HAProxyConfig `yaml:"haProxy,omitempty"` - - // RegistryRequiresAuth is computed internally and is not user-configurable. - RegistryRequiresAuth bool `yaml:"-"` } // Registry returns the resolved image registry, defaulting to @@ -81,16 +78,6 @@ func (c *RoxieConfig) UsesCustomRegistry() bool { return c.Registry() != constants.DefaultRegistry } -// NeedsPullSecrets returns whether roxie needs to set up image pull secrets itself. -// For a custom registry this relies on RegistryRequiresAuth having already been -// resolved during deploy validation (see cmd/deploy.go's deployValidate). -func (c *RoxieConfig) NeedsPullSecrets() bool { - if c.UsesCustomRegistry() { - return c.RegistryRequiresAuth - } - return c.ClusterType.NeedsDefaultRegistryPullSecrets() -} - func (c *RoxieConfig) KonfluxImagesSet() bool { return c.KonfluxImages != nil } diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 1ac97859..795a1153 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -215,7 +215,7 @@ func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { func (d *Deployer) deployCentralOperator(ctx context.Context) error { d.logger.Info("🚀 Deploying Central via Operator...") - needPullSecrets := d.config.Roxie.NeedsPullSecrets() + needPullSecrets := d.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.Central.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } @@ -822,7 +822,7 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context) error { func (d *Deployer) deploySecuredClusterOperator(ctx context.Context) error { d.logger.Info("🚀 Deploying SecuredCluster via Operator...") - needPullSecrets := d.config.Roxie.NeedsPullSecrets() + needPullSecrets := d.NeedsPullSecrets() if err := d.prepareNamespace(ctx, d.config.SecuredCluster.Namespace, needPullSecrets); err != nil { return fmt.Errorf("failed to prepare namespace: %w", err) } diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index b829f719..f8bdad20 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -61,6 +61,7 @@ type Deployer struct { portForward *portforward.Manager portForwardPID int useOperatorPullSecrets bool + registryRequiresAuth bool } type ResourceToDelete struct { @@ -300,10 +301,36 @@ func (d *Deployer) stopDetachedPortForward() { d.portForwardPID = 0 } +// ResolveRegistryAuth probes whether the configured custom registry requires +// authentication and records the result for NeedsPullSecrets to use. It's a +// no-op for the default registry, which is already known to require auth. +// Should be called once, after SetConfig and before Deploy. +func (d *Deployer) ResolveRegistryAuth(ctx context.Context) { + if !d.config.Roxie.UsesCustomRegistry() { + return + } + registry := d.config.Roxie.Registry() + d.registryRequiresAuth = d.dockerAuth.RegistryRequiresAuth(ctx, registry) + if d.registryRequiresAuth { + d.logger.Dimf("Registry %s requires authentication", registry) + } else { + d.logger.Dimf("Registry %s is public, no authentication required", registry) + } +} + +// NeedsPullSecrets reports whether roxie needs to set up image pull secrets itself. +// For a custom registry this relies on ResolveRegistryAuth having already been called. +func (d *Deployer) NeedsPullSecrets() bool { + if d.config.Roxie.UsesCustomRegistry() { + return d.registryRequiresAuth + } + return d.config.Roxie.ClusterType.NeedsDefaultRegistryPullSecrets() +} + // Deploy deploys the specified components to the cluster. func (d *Deployer) Deploy(ctx context.Context, components component.Component) error { // Prepare and verify credentials early to fail fast. - needPullSecrets := d.config.Roxie.NeedsPullSecrets() + needPullSecrets := d.NeedsPullSecrets() if needPullSecrets { if err := d.prepareCredentials(ctx); err != nil { return fmt.Errorf("failed to prepare credentials: %w", err) diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 8f563c5f..efbbb1d6 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -222,11 +222,13 @@ func (d *Deployer) resolveBundleImage(ctx context.Context, instance OperatorInst return bundleImage, nil } -func needsOperatorPullSecrets(instance OperatorInstanceConfig, roxieConfig *RoxieConfig) bool { - if roxieConfig.UsesCustomRegistry() { - return roxieConfig.RegistryRequiresAuth +// needsOperatorPullSecrets reports whether a pull secret should be created for the +// operator's own deployment namespace. +func (d *Deployer) needsOperatorPullSecrets(instance OperatorInstanceConfig) bool { + if d.config.Roxie.UsesCustomRegistry() { + return d.registryRequiresAuth } - return instance.KonfluxImagesEnabled() && roxieConfig.ClusterType.NeedsDefaultRegistryPullSecrets() + return instance.KonfluxImagesEnabled() && d.config.Roxie.ClusterType.NeedsDefaultRegistryPullSecrets() } // deployOperatorFromCSV deploys the operator from CSV into the given instance namespace. @@ -244,7 +246,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string, } serviceAccountName := deploymentSpec["service_account"].(string) - d.useOperatorPullSecrets = needsOperatorPullSecrets(instance, &d.config.Roxie) + d.useOperatorPullSecrets = d.needsOperatorPullSecrets(instance) d.logger.Info("📋 Operator deployment plan:") d.logger.Dimf(" • Namespace: %s", instance.Namespace) diff --git a/internal/deployer/operator_test.go b/internal/deployer/operator_test.go index 12a59902..a7a79f66 100644 --- a/internal/deployer/operator_test.go +++ b/internal/deployer/operator_test.go @@ -9,10 +9,11 @@ import ( func TestNeedsOperatorPullSecrets(t *testing.T) { tests := []struct { - name string - instance OperatorInstanceConfig - roxieConfig RoxieConfig - expected bool + name string + instance OperatorInstanceConfig + roxieConfig RoxieConfig + registryRequiresAuth bool + expected bool }{ { name: "default registry, non-Konflux: no pull secrets", @@ -33,37 +34,42 @@ func TestNeedsOperatorPullSecrets(t *testing.T) { expected: false, }, { - name: "private custom registry, non-Konflux: pull secrets needed", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE, RegistryRequiresAuth: true}, - expected: true, + name: "private custom registry, non-Konflux: pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, + registryRequiresAuth: true, + expected: true, }, { - name: "private custom registry is never auto-configured, even on a cluster type that auto-configures the default registry", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: true}, - expected: true, + name: "private custom registry is never auto-configured, even on a cluster type that auto-configures the default registry", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + registryRequiresAuth: true, + expected: true, }, { - name: "public custom registry: no pull secrets needed", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE, RegistryRequiresAuth: false}, - expected: false, + name: "public custom registry: no pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, + registryRequiresAuth: false, + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, needsOperatorPullSecrets(tt.instance, &tt.roxieConfig)) + d := &Deployer{config: Config{Roxie: tt.roxieConfig}, registryRequiresAuth: tt.registryRequiresAuth} + assert.Equal(t, tt.expected, d.needsOperatorPullSecrets(tt.instance)) }) } } -func TestRoxieConfig_NeedsPullSecrets(t *testing.T) { +func TestDeployer_NeedsPullSecrets(t *testing.T) { tests := []struct { - name string - roxie RoxieConfig - expected bool + name string + roxie RoxieConfig + registryRequiresAuth bool + expected bool }{ { name: "default registry on a cluster type that auto-configures credentials", @@ -76,20 +82,23 @@ func TestRoxieConfig_NeedsPullSecrets(t *testing.T) { expected: true, }, { - name: "private custom registry, even on a cluster type that auto-configures default-registry credentials", - roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: true}, - expected: true, + name: "private custom registry, even on a cluster type that auto-configures default-registry credentials", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + registryRequiresAuth: true, + expected: true, }, { - name: "public custom registry: no pull secrets needed", - roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4, RegistryRequiresAuth: false}, - expected: false, + name: "public custom registry: no pull secrets needed", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + registryRequiresAuth: false, + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, tt.roxie.NeedsPullSecrets()) + d := &Deployer{config: Config{Roxie: tt.roxie}, registryRequiresAuth: tt.registryRequiresAuth} + assert.Equal(t, tt.expected, d.NeedsPullSecrets()) }) } } From d2bb928311cc40b2a639d922da1984f94862cb83 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Sun, 16 Aug 2026 22:10:41 +0200 Subject: [PATCH 07/14] Use DefaultRoxieConfig --- cmd/deploy.go | 4 ++-- internal/deployer/acs_images.go | 2 +- internal/deployer/config.go | 13 ++----------- internal/deployer/deploy_via_operator.go | 4 ++-- internal/deployer/deployer.go | 9 +++------ internal/deployer/operator.go | 6 +++--- internal/deployer/operator_instances_test.go | 5 +++-- internal/deployer/operator_test.go | 11 ++++++----- 8 files changed, 22 insertions(+), 32 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 497ed728..53a43a04 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -286,7 +286,7 @@ func runDeploy(cmd *cobra.Command, args []string) error { d.ResolveRegistryAuth(ctx) if d.NeedsPullSecrets() { - if err := validateContainerizedCredentials(deploySettings.Roxie.Registry(), deploySettings.Roxie.ClusterType); err != nil { + if err := validateContainerizedCredentials(deploySettings.Roxie.ImageRegistry, deploySettings.Roxie.ClusterType); err != nil { return err } } @@ -494,7 +494,7 @@ func deployValidate(log *logger.Logger, components component.Component, deploySe return errors.New("running without a controlling terminal requires --envrc to be set") } - registry := deploySettings.Roxie.Registry() + registry := deploySettings.Roxie.ImageRegistry if deploySettings.Roxie.UsesCustomRegistry() { if err := validateImageRegistry(registry); err != nil { return err diff --git a/internal/deployer/acs_images.go b/internal/deployer/acs_images.go index de27c916..a35dfe74 100644 --- a/internal/deployer/acs_images.go +++ b/internal/deployer/acs_images.go @@ -6,7 +6,7 @@ import ( func imagesForConfig(config Config) []string { var images []string - imageRegistry := config.Roxie.Registry() + imageRegistry := config.Roxie.ImageRegistry for _, instance := range config.OperatorInstances() { prefix := "" diff --git a/internal/deployer/config.go b/internal/deployer/config.go index bc912416..360285cf 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -2,7 +2,6 @@ package deployer import ( "fmt" - "strings" "time" "github.com/stackrox/roxie/internal/constants" @@ -64,18 +63,9 @@ type RoxieConfig struct { HAProxy HAProxyConfig `yaml:"haProxy,omitempty"` } -// Registry returns the resolved image registry, defaulting to -// constants.DefaultRegistry when ImageRegistry is not set. -func (c *RoxieConfig) Registry() string { - if c.ImageRegistry == "" { - return constants.DefaultRegistry - } - return strings.TrimSuffix(c.ImageRegistry, "/") -} - // UsesCustomRegistry returns whether a custom image registry was configured. func (c *RoxieConfig) UsesCustomRegistry() bool { - return c.Registry() != constants.DefaultRegistry + return c.ImageRegistry != constants.DefaultRegistry } func (c *RoxieConfig) KonfluxImagesSet() bool { @@ -222,6 +212,7 @@ func NewCentralConfig() CentralConfig { func DefaultRoxieConfig() RoxieConfig { cfg := NewRoxieConfig() cfg.HAProxy.BindPort = defaultHAProxyBindPort + cfg.ImageRegistry = constants.DefaultRegistry return cfg } diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 795a1153..b3986a70 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -257,7 +257,7 @@ func (d *Deployer) isOperatorVersionCorrect(ctx context.Context, instance Operat return false } - desiredImage := instance.OperatorImage(d.config.Roxie.Registry()) + desiredImage := instance.OperatorImage(d.config.Roxie.ImageRegistry) if currentImage != desiredImage { d.logger.Info("Operator image mismatch detected:") d.logger.Infof(" Current: %s", currentImage) @@ -303,7 +303,7 @@ func (d *Deployer) ensurePullSecretExists(ctx context.Context, namespace string) return errors.New("no pull secrets available to set up on the cluster") } - pullSecretYAML := d.dockerAuth.CreatePullSecretYAMLFromCredentials(*d.dockerCreds, namespace, d.config.Roxie.Registry()) + pullSecretYAML := d.dockerAuth.CreatePullSecretYAMLFromCredentials(*d.dockerCreds, namespace, d.config.Roxie.ImageRegistry) _, err := d.runKubectl(ctx, k8s.KubectlOptions{ Args: []string{"apply", "-f", "-"}, Stdin: strings.NewReader(pullSecretYAML), diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index f8bdad20..7b6b2869 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -301,15 +301,12 @@ func (d *Deployer) stopDetachedPortForward() { d.portForwardPID = 0 } -// ResolveRegistryAuth probes whether the configured custom registry requires -// authentication and records the result for NeedsPullSecrets to use. It's a -// no-op for the default registry, which is already known to require auth. -// Should be called once, after SetConfig and before Deploy. +// ResolveRegistryAuth probes whether the configured custom registry is public or requires authentication. func (d *Deployer) ResolveRegistryAuth(ctx context.Context) { if !d.config.Roxie.UsesCustomRegistry() { return } - registry := d.config.Roxie.Registry() + registry := d.config.Roxie.ImageRegistry d.registryRequiresAuth = d.dockerAuth.RegistryRequiresAuth(ctx, registry) if d.registryRequiresAuth { d.logger.Dimf("Registry %s requires authentication", registry) @@ -385,7 +382,7 @@ func (d *Deployer) prepareCredentials(ctx context.Context) error { d.logger.Dimf("Preparing and verifying Docker credentials...") // This will retrieve and verify credentials, returning error if invalid - creds, err := d.dockerAuth.GetAndVerifyCredentials(ctx, d.config.Roxie.Registry()) + creds, err := d.dockerAuth.GetAndVerifyCredentials(ctx, d.config.Roxie.ImageRegistry) if err != nil { return err } diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index efbbb1d6..8a6422a3 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -38,7 +38,7 @@ var requiredCRDs = []string{ // deployOperatorNonOLM deploys one RHACS operator instance without OLM. func (d *Deployer) deployOperatorNonOLM(ctx context.Context, instance OperatorInstanceConfig) error { d.logger.Infof("Operator tag: %s (namespace %s)", instance.Version, instance.Namespace) - bundleImage, err := d.resolveBundleImage(ctx, instance, d.config.Roxie.Registry()) + bundleImage, err := d.resolveBundleImage(ctx, instance, d.config.Roxie.ImageRegistry) if err != nil { return fmt.Errorf("resolving operator bundle image: %w", err) } @@ -174,7 +174,7 @@ func (d *Deployer) ensureCRDsInstalled(ctx context.Context) error { if len(missing) > 0 { crdInstance := d.config.NewestOperatorInstance() - bundleImage, err := d.resolveBundleImage(ctx, crdInstance, d.config.Roxie.Registry()) + bundleImage, err := d.resolveBundleImage(ctx, crdInstance, d.config.Roxie.ImageRegistry) if err != nil { return fmt.Errorf("resolving operator bundle image: %w", err) } @@ -482,7 +482,7 @@ func (d *Deployer) createDeploymentFromCSV(ctx context.Context, instance Operato return fmt.Errorf("extracting manager container from operator pod spec: %w", err) } - operatorImage := instance.OperatorImage(d.config.Roxie.Registry()) + operatorImage := instance.OperatorImage(d.config.Roxie.ImageRegistry) podSpec["serviceAccountName"] = deploymentSpec["service_account"] if current, _ := managerContainer["image"].(string); current != operatorImage { d.logger.Infof("Rewriting operator image to %s", operatorImage) diff --git a/internal/deployer/operator_instances_test.go b/internal/deployer/operator_instances_test.go index 5ef13c47..aa6313fb 100644 --- a/internal/deployer/operator_instances_test.go +++ b/internal/deployer/operator_instances_test.go @@ -3,6 +3,7 @@ package deployer import ( "testing" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/imagetag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -222,7 +223,7 @@ func TestNewestOperatorVersion(t *testing.T) { func TestImagesForConfig_MixedVersions(t *testing.T) { cfg := Config{ - Roxie: RoxieConfig{Version: "4.9.0"}, + Roxie: RoxieConfig{Version: "4.9.0", ImageRegistry: constants.DefaultRegistry}, Central: CentralConfig{Operator: OperatorInstanceConfig{Version: "4.8.0"}}, SecuredCluster: SecuredClusterConfig{Operator: OperatorInstanceConfig{Version: "4.9.0"}}, } @@ -238,7 +239,7 @@ func TestImagesForConfig_MixedVersions(t *testing.T) { func TestImagesForConfig_SingleVersionUnchanged(t *testing.T) { cfg := Config{ - Roxie: RoxieConfig{Version: "4.9.0"}, + Roxie: RoxieConfig{Version: "4.9.0", ImageRegistry: constants.DefaultRegistry}, Operator: OperatorConfig{ OperatorInstanceConfig: OperatorInstanceConfig{Version: "4.9.0"}, }, diff --git a/internal/deployer/operator_test.go b/internal/deployer/operator_test.go index a7a79f66..d99d5b0f 100644 --- a/internal/deployer/operator_test.go +++ b/internal/deployer/operator_test.go @@ -3,6 +3,7 @@ package deployer import ( "testing" + "github.com/stackrox/roxie/internal/constants" "github.com/stackrox/roxie/internal/types" "github.com/stretchr/testify/assert" ) @@ -18,19 +19,19 @@ func TestNeedsOperatorPullSecrets(t *testing.T) { { name: "default registry, non-Konflux: no pull secrets", instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + roxieConfig: RoxieConfig{ImageRegistry: constants.DefaultRegistry, ClusterType: types.ClusterTypeGKE}, expected: false, }, { name: "Konflux images: pull secrets needed", instance: OperatorInstanceConfig{KonfluxImages: new(true)}, - roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + roxieConfig: RoxieConfig{ImageRegistry: constants.DefaultRegistry, ClusterType: types.ClusterTypeGKE}, expected: true, }, { name: "Konflux images on a cluster type that auto-configures default-registry credentials: no pull secrets", instance: OperatorInstanceConfig{KonfluxImages: new(true)}, - roxieConfig: RoxieConfig{ClusterType: types.ClusterTypeInfraOpenShift4}, + roxieConfig: RoxieConfig{ImageRegistry: constants.DefaultRegistry, ClusterType: types.ClusterTypeInfraOpenShift4}, expected: false, }, { @@ -73,12 +74,12 @@ func TestDeployer_NeedsPullSecrets(t *testing.T) { }{ { name: "default registry on a cluster type that auto-configures credentials", - roxie: RoxieConfig{ClusterType: types.ClusterTypeInfraOpenShift4}, + roxie: RoxieConfig{ImageRegistry: constants.DefaultRegistry, ClusterType: types.ClusterTypeInfraOpenShift4}, expected: false, }, { name: "default registry on a cluster type that doesn't auto-configure credentials", - roxie: RoxieConfig{ClusterType: types.ClusterTypeGKE}, + roxie: RoxieConfig{ImageRegistry: constants.DefaultRegistry, ClusterType: types.ClusterTypeGKE}, expected: true, }, { From bdd6e9f24d2c450e645eee1fa1320113ae5c1179 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Sun, 16 Aug 2026 22:25:46 +0200 Subject: [PATCH 08/14] Rename test --- tests/e2e/custom_registry_test.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/e2e/custom_registry_test.go b/tests/e2e/custom_registry_test.go index 52ff0c0e..b0774971 100644 --- a/tests/e2e/custom_registry_test.go +++ b/tests/e2e/custom_registry_test.go @@ -3,7 +3,6 @@ package e2e import ( - "os" "os/exec" "strings" "testing" @@ -11,29 +10,24 @@ import ( "github.com/stretchr/testify/require" ) -// TestDeployWithStackroxIORegistry verifies that roxie can deploy Central using +// TestCentralDeployWithStackroxIORegistry verifies that roxie can deploy Central using // the public quay.io/stackrox-io registry instead of the default quay.io/rhacs-eng. -func TestDeployWithStackroxIORegistry(t *testing.T) { +func TestCentralDeployWithStackroxIORegistry(t *testing.T) { dumpClusterStateOnFailure(t) const stackroxIORegistry = "quay.io/stackrox-io" - envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") - require.NoError(t, err) - envrcPath := envrcFile.Name() - envrcFile.Close() - t.Log("=== Deploying central with quay.io/stackrox-io registry ===") args := append([]string{ roxieBinary, "deploy", "--early-readiness", "central", "--set", "roxie.imageRegistry=" + stackroxIORegistry, - "--envrc", envrcPath, + "--envrc", "/dev/null", }, commonDeployArgs...) runCommand(t, deployTimeout, nil, args...) - verifyCentralInstalled(t, centralNamespace) verifyOperatorDeploymentExists(t, operatorSystemNamespace) verifyOperatorImageRegistry(t, operatorSystemNamespace, stackroxIORegistry) + verifyCentralInstalled(t, centralNamespace) t.Log("=== Cleaning up ===") teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"} From 0b7a10dc99eb78264ae2937e46c42c472fcc83b4 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 10:40:40 +0200 Subject: [PATCH 09/14] Return error from RegistryRequiresAuth --- internal/deployer/deployer.go | 8 ++++++-- internal/dockerauth/dockerauth.go | 15 ++++++++------- internal/dockerauth/dockerauth_test.go | 9 ++++++++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index 7b6b2869..ef78aeea 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -307,12 +307,16 @@ func (d *Deployer) ResolveRegistryAuth(ctx context.Context) { return } registry := d.config.Roxie.ImageRegistry - d.registryRequiresAuth = d.dockerAuth.RegistryRequiresAuth(ctx, registry) - if d.registryRequiresAuth { + requiresAuth, err := d.dockerAuth.RegistryRequiresAuth(ctx, registry) + if err != nil { + d.logger.Warningf("Could not determine if %s requires auth, will require credentials: %v", registry, err) + requiresAuth = true + } else if requiresAuth { d.logger.Dimf("Registry %s requires authentication", registry) } else { d.logger.Dimf("Registry %s is public, no authentication required", registry) } + d.registryRequiresAuth = requiresAuth } // NeedsPullSecrets reports whether roxie needs to set up image pull secrets itself. diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index ac7c7863..d56bd2bd 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -213,34 +213,35 @@ func (d *DockerAuth) VerifyCredentials(ctx context.Context, username, password, // RegistryRequiresAuth makes a best-effort check for whether registry requires // authentication to pull images, by sending a single anonymous tags-list // request against a well-known repository path. Anything short of a confirmed -// successful response fails safe by reporting that auth is required. -func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) bool { +// successful response fails safe by reporting that auth is required, alongside +// an error explaining why the check was inconclusive. +func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) (bool, error) { host, orgPath := splitRegistryHost(registry) reg, err := name.NewRegistry(host) if err != nil { - return true + return true, fmt.Errorf("invalid registry host %q: %w", host, err) } repo := reg.Repo(orgPath, "main") tr, err := transport.NewWithContext(ctx, reg, authn.Anonymous, http.DefaultTransport, []string{repo.Scope("pull")}) if err != nil { - return true + return true, fmt.Errorf("negotiating anonymous access to %s: %w", host, err) } url := fmt.Sprintf("%s://%s/v2/%s/tags/list?n=1", repo.Scheme(), repo.RegistryStr(), repo.RepositoryStr()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { - return true + return true, fmt.Errorf("building request for %s: %w", host, err) } resp, err := (&http.Client{Transport: tr}).Do(req) if err != nil { - return true + return true, fmt.Errorf("checking %s: %w", host, err) } defer resp.Body.Close() - return resp.StatusCode < 200 || resp.StatusCode >= 300 + return resp.StatusCode < 200 || resp.StatusCode >= 300, nil } // CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index f9c21ea3..3d11a24f 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -112,6 +112,7 @@ func TestRegistryRequiresAuth(t *testing.T) { tokenStatus int // status the token endpoint returns, if challenged tagsListStatus int // status the tags-list request returns expectedRequires bool + expectErr bool }{ { name: "no auth mechanism: public", @@ -148,6 +149,7 @@ func TestRegistryRequiresAuth(t *testing.T) { challengeAuth: true, tokenStatus: http.StatusUnauthorized, expectedRequires: true, + expectErr: true, }, { name: "tags-list request fails with a server error", @@ -191,8 +193,13 @@ func TestRegistryRequiresAuth(t *testing.T) { registryAddr = strings.TrimPrefix(server.URL, "http://") da := &DockerAuth{logger: logger.New()} - requiresAuth := da.RegistryRequiresAuth(context.Background(), registryAddr+"/some-org") + requiresAuth, err := da.RegistryRequiresAuth(context.Background(), registryAddr+"/some-org") assert.Equal(t, tt.expectedRequires, requiresAuth) + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } }) } } From ae44c99078cef990d421abda64e3cc6a8a0476bf Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 10:57:34 +0200 Subject: [PATCH 10/14] Handle http codes explicitly --- internal/dockerauth/dockerauth.go | 8 +++++++- internal/dockerauth/dockerauth_test.go | 7 +++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index d56bd2bd..e840a50e 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -241,7 +241,13 @@ func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) } defer resp.Body.Close() - return resp.StatusCode < 200 || resp.StatusCode >= 300, nil + if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices { + return false, nil + } + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound { + return true, nil + } + return true, fmt.Errorf("unexpected status %s from %s", resp.Status, host) } // CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index 3d11a24f..eae465f1 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -152,14 +152,13 @@ func TestRegistryRequiresAuth(t *testing.T) { expectErr: true, }, { - name: "tags-list request fails with a server error", - // A transient 5xx doesn't tell us whether the registry is public or - // private, so this fails safe by reporting that auth is required, - // rather than treating it as confirmed "public". + name: "tags-list request returns an unexpected server error", + // A transient 5xx doesn't tell us whether the registry is public or private. challengeAuth: true, tokenStatus: http.StatusOK, tagsListStatus: http.StatusInternalServerError, expectedRequires: true, + expectErr: true, }, } From e55f1f8994ef8299133527fccaa2c4c56a5a90d6 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 11:16:34 +0200 Subject: [PATCH 11/14] Rename isOperatorVersionCorrect to isOperatorImageCorrect --- internal/deployer/deploy_via_operator.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index b3986a70..e0a85d3a 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -130,11 +130,11 @@ func (d *Deployer) ensureOperatorInstanceNonOLM(ctx context.Context, instance Op needsTeardown := false if exists { - if d.isOperatorVersionCorrect(ctx, instance) { - d.logger.Infof("✓ Operator already deployed with correct version in namespace %s", instance.Namespace) + if d.isOperatorImageCorrect(ctx, instance) { + d.logger.Infof("✓ Operator already deployed with correct image in namespace %s", instance.Namespace) return nil } - d.logger.Infof("🔄 Operator version mismatch in namespace %s, redeploying...", instance.Namespace) + d.logger.Infof("🔄 Operator image mismatch in namespace %s, redeploying...", instance.Namespace) needsTeardown = true needsDeployment = true } @@ -181,10 +181,10 @@ func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { Namespace: operatorNamespaceSystem, EnvVars: d.config.Operator.EnvVars, } - if d.isOperatorVersionCorrect(ctx, instance) { - d.logger.Info("✓ Operator already deployed with correct version") + if d.isOperatorImageCorrect(ctx, instance) { + d.logger.Info("✓ Operator already deployed with correct image") } else { - d.logger.Info("🔄 Operator version mismatch, redeploying...") + d.logger.Info("🔄 Operator image mismatch, redeploying...") needsTeardown = true needsDeployment = true } @@ -248,9 +248,9 @@ func (d *Deployer) deployCentralOperator(ctx context.Context) error { return d.configureCentralEndpoint(ctx) } -// isOperatorVersionCorrect checks if the deployed operator matches the desired +// isOperatorImageCorrect checks if the deployed operator matches the desired // image, comparing the full reference (registry, repository, and tag). -func (d *Deployer) isOperatorVersionCorrect(ctx context.Context, instance OperatorInstanceConfig) bool { +func (d *Deployer) isOperatorImageCorrect(ctx context.Context, instance OperatorInstanceConfig) bool { currentImage, err := d.getDeployedOperatorImage(ctx, instance.Namespace) if err != nil { d.logger.Warningf("Could not retrieve operator image: %v", err) From df8a90bf0109d164ac213a847645dc82049e1292 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 12:30:06 +0200 Subject: [PATCH 12/14] Memoize custom registry auth probe --- cmd/deploy.go | 2 - internal/deployer/deployer.go | 34 ++++++++------ internal/deployer/operator.go | 2 +- internal/deployer/operator_test.go | 74 ++++++++++++++++-------------- 4 files changed, 61 insertions(+), 51 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 53a43a04..a67205c9 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -283,8 +283,6 @@ func runDeploy(cmd *cobra.Command, args []string) error { d.SetVerbose(verbose) d.SetConfig(deploySettings) - d.ResolveRegistryAuth(ctx) - if d.NeedsPullSecrets() { if err := validateContainerizedCredentials(deploySettings.Roxie.ImageRegistry, deploySettings.Roxie.ClusterType); err != nil { return err diff --git a/internal/deployer/deployer.go b/internal/deployer/deployer.go index ef78aeea..5742d48f 100644 --- a/internal/deployer/deployer.go +++ b/internal/deployer/deployer.go @@ -54,14 +54,14 @@ type Deployer struct { config Config // State - centralEndpoint string - centralPassword string - roxCACertFile string - tempDir string - portForward *portforward.Manager - portForwardPID int - useOperatorPullSecrets bool - registryRequiresAuth bool + centralEndpoint string + centralPassword string + roxCACertFile string + tempDir string + portForward *portforward.Manager + portForwardPID int + useOperatorPullSecrets bool + customRegistryAuthRequired *bool } type ResourceToDelete struct { @@ -301,12 +301,18 @@ func (d *Deployer) stopDetachedPortForward() { d.portForwardPID = 0 } -// ResolveRegistryAuth probes whether the configured custom registry is public or requires authentication. -func (d *Deployer) ResolveRegistryAuth(ctx context.Context) { +// customRegistryRequiresAuth reports whether the configured custom registry +// requires authentication, probing it once and caching the result. +func (d *Deployer) customRegistryRequiresAuth() bool { + if d.customRegistryAuthRequired != nil { + return *d.customRegistryAuthRequired + } if !d.config.Roxie.UsesCustomRegistry() { - return + return false } registry := d.config.Roxie.ImageRegistry + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() requiresAuth, err := d.dockerAuth.RegistryRequiresAuth(ctx, registry) if err != nil { d.logger.Warningf("Could not determine if %s requires auth, will require credentials: %v", registry, err) @@ -316,14 +322,14 @@ func (d *Deployer) ResolveRegistryAuth(ctx context.Context) { } else { d.logger.Dimf("Registry %s is public, no authentication required", registry) } - d.registryRequiresAuth = requiresAuth + d.customRegistryAuthRequired = &requiresAuth + return requiresAuth } // NeedsPullSecrets reports whether roxie needs to set up image pull secrets itself. -// For a custom registry this relies on ResolveRegistryAuth having already been called. func (d *Deployer) NeedsPullSecrets() bool { if d.config.Roxie.UsesCustomRegistry() { - return d.registryRequiresAuth + return d.customRegistryRequiresAuth() } return d.config.Roxie.ClusterType.NeedsDefaultRegistryPullSecrets() } diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 8a6422a3..cbdb5f53 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -226,7 +226,7 @@ func (d *Deployer) resolveBundleImage(ctx context.Context, instance OperatorInst // operator's own deployment namespace. func (d *Deployer) needsOperatorPullSecrets(instance OperatorInstanceConfig) bool { if d.config.Roxie.UsesCustomRegistry() { - return d.registryRequiresAuth + return d.NeedsPullSecrets() } return instance.KonfluxImagesEnabled() && d.config.Roxie.ClusterType.NeedsDefaultRegistryPullSecrets() } diff --git a/internal/deployer/operator_test.go b/internal/deployer/operator_test.go index d99d5b0f..e5b1d64a 100644 --- a/internal/deployer/operator_test.go +++ b/internal/deployer/operator_test.go @@ -10,11 +10,11 @@ import ( func TestNeedsOperatorPullSecrets(t *testing.T) { tests := []struct { - name string - instance OperatorInstanceConfig - roxieConfig RoxieConfig - registryRequiresAuth bool - expected bool + name string + instance OperatorInstanceConfig + roxieConfig RoxieConfig + customRegistryAuthRequired *bool + expected bool }{ { name: "default registry, non-Konflux: no pull secrets", @@ -35,31 +35,34 @@ func TestNeedsOperatorPullSecrets(t *testing.T) { expected: false, }, { - name: "private custom registry, non-Konflux: pull secrets needed", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, - registryRequiresAuth: true, - expected: true, + name: "private custom registry, non-Konflux: pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, + customRegistryAuthRequired: new(true), + expected: true, }, { - name: "private custom registry is never auto-configured, even on a cluster type that auto-configures the default registry", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, - registryRequiresAuth: true, - expected: true, + name: "private custom registry is never auto-configured, even on a cluster type that auto-configures the default registry", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + customRegistryAuthRequired: new(true), + expected: true, }, { - name: "public custom registry: no pull secrets needed", - instance: OperatorInstanceConfig{}, - roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, - registryRequiresAuth: false, - expected: false, + name: "public custom registry: no pull secrets needed", + instance: OperatorInstanceConfig{}, + roxieConfig: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeGKE}, + customRegistryAuthRequired: new(false), + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - d := &Deployer{config: Config{Roxie: tt.roxieConfig}, registryRequiresAuth: tt.registryRequiresAuth} + d := &Deployer{ + config: Config{Roxie: tt.roxieConfig}, + customRegistryAuthRequired: tt.customRegistryAuthRequired, + } assert.Equal(t, tt.expected, d.needsOperatorPullSecrets(tt.instance)) }) } @@ -67,10 +70,10 @@ func TestNeedsOperatorPullSecrets(t *testing.T) { func TestDeployer_NeedsPullSecrets(t *testing.T) { tests := []struct { - name string - roxie RoxieConfig - registryRequiresAuth bool - expected bool + name string + roxie RoxieConfig + customRegistryAuthRequired *bool + expected bool }{ { name: "default registry on a cluster type that auto-configures credentials", @@ -83,22 +86,25 @@ func TestDeployer_NeedsPullSecrets(t *testing.T) { expected: true, }, { - name: "private custom registry, even on a cluster type that auto-configures default-registry credentials", - roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, - registryRequiresAuth: true, - expected: true, + name: "private custom registry, even on a cluster type that auto-configures default-registry credentials", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + customRegistryAuthRequired: new(true), + expected: true, }, { - name: "public custom registry: no pull secrets needed", - roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, - registryRequiresAuth: false, - expected: false, + name: "public custom registry: no pull secrets needed", + roxie: RoxieConfig{ImageRegistry: "quay.io/stackrox-io", ClusterType: types.ClusterTypeInfraOpenShift4}, + customRegistryAuthRequired: new(false), + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - d := &Deployer{config: Config{Roxie: tt.roxie}, registryRequiresAuth: tt.registryRequiresAuth} + d := &Deployer{ + config: Config{Roxie: tt.roxie}, + customRegistryAuthRequired: tt.customRegistryAuthRequired, + } assert.Equal(t, tt.expected, d.NeedsPullSecrets()) }) } From cbb709ff191ee1837b7e246cde9dec04d7241aff Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 13:52:35 +0200 Subject: [PATCH 13/14] e2e test - verify Central registry too --- tests/e2e/custom_registry_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/e2e/custom_registry_test.go b/tests/e2e/custom_registry_test.go index b0774971..03e84883 100644 --- a/tests/e2e/custom_registry_test.go +++ b/tests/e2e/custom_registry_test.go @@ -26,8 +26,9 @@ func TestCentralDeployWithStackroxIORegistry(t *testing.T) { runCommand(t, deployTimeout, nil, args...) verifyOperatorDeploymentExists(t, operatorSystemNamespace) - verifyOperatorImageRegistry(t, operatorSystemNamespace, stackroxIORegistry) verifyCentralInstalled(t, centralNamespace) + verifyDeploymentImageRegistry(t, operatorSystemNamespace, operatorDeploymentName, stackroxIORegistry) + verifyDeploymentImageRegistry(t, centralNamespace, "central", stackroxIORegistry) t.Log("=== Cleaning up ===") teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"} @@ -36,18 +37,18 @@ func TestCentralDeployWithStackroxIORegistry(t *testing.T) { verifyCentralNotInstalled(t, centralNamespace) } -// verifyOperatorImageRegistry asserts that the operator deployment's image is +// verifyDeploymentImageRegistry asserts that the given deployment's image is // hosted on the expected registry. -func verifyOperatorImageRegistry(t *testing.T, namespace, expectedRegistry string) { +func verifyDeploymentImageRegistry(t *testing.T, namespace, deployment, expectedRegistry string) { t.Helper() - cmd := exec.Command("kubectl", "get", "deployment", operatorDeploymentName, "-n", namespace, + cmd := exec.Command("kubectl", "get", "deployment", deployment, "-n", namespace, "-o", "jsonpath={.spec.template.spec.containers[0].image}") output, err := cmd.Output() - require.NoErrorf(t, err, "Failed to get operator image in namespace %s", namespace) + require.NoErrorf(t, err, "Failed to get image for deployment %s in namespace %s", deployment, namespace) image := strings.TrimSpace(string(output)) require.Truef(t, strings.HasPrefix(image, expectedRegistry+"/"), - "Expected operator image to be pulled from %s, got: %s", expectedRegistry, image) - t.Logf("✓ Operator image %s uses registry %s", image, expectedRegistry) + "Expected %s image to be pulled from %s, got: %s", deployment, expectedRegistry, image) + t.Logf("✓ %s image %s uses registry %s", deployment, image, expectedRegistry) } From cf233729e3f0e2b0fce5316d4c3cd51a3a44ae7f Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 17 Aug 2026 15:54:43 +0200 Subject: [PATCH 14/14] Fix bug in RegistryRequiresAuth --- internal/dockerauth/dockerauth.go | 10 +++++++++- internal/dockerauth/dockerauth_test.go | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/dockerauth/dockerauth.go b/internal/dockerauth/dockerauth.go index e840a50e..3e4a8787 100644 --- a/internal/dockerauth/dockerauth.go +++ b/internal/dockerauth/dockerauth.go @@ -226,6 +226,10 @@ func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) tr, err := transport.NewWithContext(ctx, reg, authn.Anonymous, http.DefaultTransport, []string{repo.Scope("pull")}) if err != nil { + var te *transport.Error + if errors.As(err, &te) && requiresAuth(te.StatusCode) { + return true, nil + } return true, fmt.Errorf("negotiating anonymous access to %s: %w", host, err) } @@ -244,12 +248,16 @@ func (d *DockerAuth) RegistryRequiresAuth(ctx context.Context, registry string) if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices { return false, nil } - if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound { + if requiresAuth(resp.StatusCode) { return true, nil } return true, fmt.Errorf("unexpected status %s from %s", resp.Status, host) } +func requiresAuth(code int) bool { + return code == http.StatusUnauthorized || code == http.StatusForbidden || code == http.StatusNotFound +} + // CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from // verified credentials, scoped to the host of the given image registry func (d *DockerAuth) CreatePullSecretYAMLFromCredentials(creds Credentials, namespace, registry string) string { diff --git a/internal/dockerauth/dockerauth_test.go b/internal/dockerauth/dockerauth_test.go index eae465f1..ec6a49f7 100644 --- a/internal/dockerauth/dockerauth_test.go +++ b/internal/dockerauth/dockerauth_test.go @@ -149,7 +149,6 @@ func TestRegistryRequiresAuth(t *testing.T) { challengeAuth: true, tokenStatus: http.StatusUnauthorized, expectedRequires: true, - expectErr: true, }, { name: "tags-list request returns an unexpected server error",