Merge pull request #7 from stackrox/mc/quay-cred-test

Refactor docker credential fetching to fail fast

Author: mclasmeier

pkg/deployer/deploy_via_operator.go

@@ -157,20 +157,18 @@ func (d *Deployer) prepareNamespace(ctx context.Context, namespace string) error
 
 	if env.CurrentClusterType != env.InfraOpenShift4 {
 		if err := d.ensurePullSecretExists(ctx, namespace); err != nil {
-			return fmt.Errorf("could not create pull secret: %w", err)
+			return fmt.Errorf("ensuring image pull secret exists: %w", err)
 		}
 	}
 
 	return nil
 }
 
 func (d *Deployer) ensurePullSecretExists(ctx context.Context, namespace string) error {
-	pullSecretYAML, err := d.dockerAuth.CreatePullSecretYAML(namespace)
-	if err != nil {
-		return fmt.Errorf("could not create pull secret: %w", err)
-	}
+	// Assemble pull secret YAML from pre-verified credentials
+	pullSecretYAML := d.dockerAuth.CreatePullSecretYAMLFromCredentials(d.dockerCreds, namespace)
 
-	_, err = d.runKubectl(ctx, KubectlOptions{
+	_, err := d.runKubectl(ctx, KubectlOptions{
 		Args:  []string{"apply", "-f", "-"},
 		Stdin: strings.NewReader(pullSecretYAML),
 	})

pkg/deployer/deployer.go

@@ -54,6 +54,7 @@ type Deployer struct {
 	useOLM                 bool
 	verbose                bool
 	earlyReadiness         bool
+	dockerCreds            *dockerauth.Credentials
 }
 
 func New(log *logger.Logger, overrideFile string, overrideSetExpressions []string) (*Deployer, error) {
@@ -140,6 +141,11 @@ func (d *Deployer) Deploy(ctx context.Context, component, resources, exposure st
 	d.portForwardEnabled = adjustedPortForward
 	d.exposure = exposure
 
+	// Prepare and verify credentials early to fail fast
+	if err := d.prepareCredentials(); err != nil {
+		return fmt.Errorf("failed to prepare credentials: %w", err)
+	}
+
 	d.logger.Infof("Initiating deployment of %s", formatComponentName(component))
 
 	switch component {
@@ -157,6 +163,24 @@ func (d *Deployer) Deploy(ctx context.Context, component, resources, exposure st
 	}
 }
 
+// prepareCredentials prepares and verifies Docker credentials early to fail fast.
+// The verified credentials are stored for later use.
+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()
+	if err != nil {
+		return err
+	}
+
+	// Store the verified credentials
+	d.dockerCreds = creds
+
+	d.logger.Dimf("Docker credentials verified successfully")
+	return nil
+}
+
 func (d *Deployer) deployCentral(ctx context.Context, resources, exposure string) error {
 	if d.namespaceExists(d.centralNamespace) {
 		d.logger.Info("Existing Central deployment found, tearing down...")

pkg/dockerauth/dockerauth.go

@@ -19,7 +19,8 @@ const (
 
 // DockerAuth handles Docker authentication and pull secret management.
 type DockerAuth struct {
-	logger *logger.Logger
+	logger               *logger.Logger
+	skipCredVerification bool
 }
 
 // DockerConfig represents Docker configuration structure.
@@ -40,26 +41,33 @@ type CredentialData struct {
 	Secret   string `json:"Secret"`
 }
 
+// Credentials represents verified Docker credentials.
+type Credentials struct {
+	Username string
+	Password string
+}
+
 // New creates a new DockerAuth instance.
 func New(log *logger.Logger) *DockerAuth {
 	return &DockerAuth{
 		logger: log,
 	}
 }
 
-// GetDockerAuthString generates Docker authentication string for image pull secrets
-func (d *DockerAuth) GetDockerAuthString(_, _ string) (string, error) {
+// GetAndVerifyCredentials retrieves and verifies Docker credentials.
+// This should be called early to fail fast if credentials are invalid.
+func (d *DockerAuth) GetAndVerifyCredentials() (*Credentials, error) {
 	var username, password string
 
 	// Try environment variables first.
 	username = os.Getenv("REGISTRY_USERNAME")
 	password = os.Getenv("REGISTRY_PASSWORD")
 
 	if username != "" && password == "" {
-		return "", errors.New("REGISTRY_USERNAME set but REGISTRY_PASSWORD is empty")
+		return nil, errors.New("REGISTRY_USERNAME set but REGISTRY_PASSWORD is empty")
 	}
 	if username == "" && password != "" {
-		return "", errors.New("REGISTRY_PASSWORD set but REGISTRY_USERNAME is empty")
+		return nil, errors.New("REGISTRY_PASSWORD set but REGISTRY_USERNAME is empty")
 	}
 
 	if username == "" {
@@ -70,31 +78,26 @@ func (d *DockerAuth) GetDockerAuthString(_, _ string) (string, error) {
 			var err error
 			username, password, err = d.getCredentialsFromDockerConfig(dockerConfigPath)
 			if err != nil {
-				return "", err
+				return nil, err
 			}
 		}
 	}
 
 	if username == "" || password == "" {
-		return "", errors.New("no Docker credentials found")
-	}
-
-	// Create auth string.
-	authString := fmt.Sprintf("%s:%s", username, password)
-	encodedAuth := base64.StdEncoding.EncodeToString([]byte(authString))
-
-	dockerConfig := DockerConfig{
-		Auths: map[string]AuthEntry{
-			acsImageRegistry: {Auth: encodedAuth},
-		},
+		return nil, errors.New("no Docker credentials found")
 	}
 
-	jsonData, err := json.Marshal(dockerConfig)
-	if err != nil {
-		return "", fmt.Errorf("failed to marshal Docker config: %w", err)
+	// Verify credentials.
+	if !d.skipCredVerification {
+		if err := d.VerifyCredentials(username, password); err != nil {
+			return nil, fmt.Errorf("credentials are invalid: %w", err)
+		}
 	}
 
-	return string(jsonData), nil
+	return &Credentials{
+		Username: username,
+		Password: password,
+	}, nil
 }
 
 // getCredentialsFromDockerConfig extracts credentials from existing Docker config.
@@ -172,14 +175,60 @@ func (d *DockerAuth) getCredentialFromHelper(helperName, registry string) (*Cred
 	return &credData, nil
 }
 
-// CreatePullSecretYAML creates Kubernetes pull secret YAML.
-func (d *DockerAuth) CreatePullSecretYAML(namespace string) (string, error) {
-	dockerConfigJSON, err := d.GetDockerAuthString("", "")
+// VerifyCredentials attempts to verify that the credentials work by making a request to the registry.
+// 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 {
+	// 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
+	// This mimics what kubelet does when pulling images - it requests a token with pull scope
+	// for the specific repository.
+	repository := "rhacs-eng/main"
+	authURL := fmt.Sprintf("https://%s/v2/auth?service=%s&scope=repository:%s:pull",
+		acsImageRegistry, acsImageRegistry, repository)
+
+	cmd := exec.Command("curl", "-s", "-f",
+		"-H", fmt.Sprintf("Authorization: Basic %s", encodedAuth),
+		authURL)
+
+	output, err := cmd.CombinedOutput()
 	if err != nil {
-		return "", err
+		d.logger.Warningf("Failed to verify credentials for %s: %v", acsImageRegistry, err)
+		d.logger.Dimf("Verification output: %s", string(output))
+		return fmt.Errorf("credential verification failed for %s: %w", acsImageRegistry, 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)
+	}
+
+	if _, ok := tokenResponse["token"]; !ok {
+		return fmt.Errorf("credential verification failed: no token received from %s", acsImageRegistry)
+	}
+
+	d.logger.Dimf("Successfully verified credentials for %s (repository: %s)", acsImageRegistry, repository)
+	return nil
+}
+
+// CreatePullSecretYAMLFromCredentials creates Kubernetes pull secret YAML from verified credentials.
+func (d *DockerAuth) CreatePullSecretYAMLFromCredentials(creds *Credentials, namespace string) string {
+	// 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},
+		},
 	}
 
-	encodedConfig := base64.StdEncoding.EncodeToString([]byte(dockerConfigJSON))
+	jsonData, _ := json.Marshal(dockerConfig)
+	encodedConfig := base64.StdEncoding.EncodeToString(jsonData)
 
 	secretYAML := fmt.Sprintf(`apiVersion: v1
 kind: Secret
@@ -191,5 +240,5 @@ data:
   .dockerconfigjson: %s
 `, namespace, encodedConfig)
 
-	return secretYAML, nil
+	return secretYAML
 }

pkg/dockerauth/dockerauth_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/stackrox/roxie/pkg/logger"
 )
 
-func TestCreatePullSecretYAMLFromEnv(t *testing.T) {
+func TestGetAndVerifyCredentialsFromEnv(t *testing.T) {
 	// Set environment variables for test
 	os.Setenv("REGISTRY_USERNAME", "user")
 	os.Setenv("REGISTRY_PASSWORD", "pass")
@@ -21,12 +21,23 @@ func TestCreatePullSecretYAMLFromEnv(t *testing.T) {
 
 	log := logger.New()
 	da := New(log)
+	da.skipCredVerification = true // Skip verification in tests
 
-	yamlText, err := da.CreatePullSecretYAML("ns")
+	creds, err := da.GetAndVerifyCredentials()
 	if err != nil {
-		t.Fatalf("CreatePullSecretYAML failed: %v", err)
+		t.Fatalf("GetAndVerifyCredentials failed: %v", err)
 	}
 
+	if creds.Username != "user" {
+		t.Errorf("Expected username 'user', got '%s'", creds.Username)
+	}
+	if creds.Password != "pass" {
+		t.Errorf("Expected password 'pass', got '%s'", creds.Password)
+	}
+
+	// Test creating YAML from credentials
+	yamlText := da.CreatePullSecretYAMLFromCredentials(creds, "ns")
+
 	// Verify YAML structure
 	if !strings.Contains(yamlText, "apiVersion: v1") {
 		t.Error("YAML should contain 'apiVersion: v1'")
@@ -71,7 +82,7 @@ func TestCreatePullSecretYAMLFromEnv(t *testing.T) {
 	}
 }
 
-func TestCreatePullSecretYAMLNoCredentials(t *testing.T) {
+func TestGetAndVerifyCredentialsNoCredentials(t *testing.T) {
 	// Ensure no credentials are set
 	os.Unsetenv("REGISTRY_USERNAME")
 	os.Unsetenv("REGISTRY_PASSWORD")
@@ -95,8 +106,9 @@ func TestCreatePullSecretYAMLNoCredentials(t *testing.T) {
 
 	log := logger.New()
 	da := New(log)
+	da.skipCredVerification = true // Skip verification in tests
 
-	_, err := da.CreatePullSecretYAML("ns")
+	_, err := da.GetAndVerifyCredentials()
 	if err == nil {
 		t.Error("Expected error when no credentials are available")
 	}