Skip to content

Commit e5a66f7

Browse files
GrimmiMeloniclaude
andcommitted
refactor: unify local image handling and fix operator tag usage
Consolidates duplicate code and improves type safety in local image detection and CSV patching: - Extract shared image list logic into getExpectedImages() helper to eliminate duplication between detection and loading - Fix bug where stackrox-operator incorrectly used mainTag instead of operatorTag - Replace generic map navigation in CSV patching with type-safe clusterServiceVersion struct - Unify shouldSkipCredentialVerification() and shouldSkipImagePullSecrets() into hasAllImagesLocally() - Change CheckLocalImage to idiomatic (value, bool, error) return pattern - Make ExtractKindClusterName and helper functions private - Remove unused test cases and helper functions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f90b107 commit e5a66f7

11 files changed

Lines changed: 139 additions & 262 deletions

File tree

README.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,13 @@ When deploying to a kind cluster, roxie:
106106

107107
- kind cluster (context name must start with "kind")
108108
- podman with images built locally
109-
- Images tagged with either:
110-
- `localhost/stackrox/<image>:<tag>`
111-
- `quay.io/<branding-org>/<image>:<tag>`
109+
- Images tagged with `quay.io/<branding-org>/<image>:<tag>`
112110

113111
### Supported Images
114112

115-
- main
113+
- main, central-db
116114
- scanner, scanner-db
117115
- scanner-v4-db, scanner-v4-indexer, scanner-v4-matcher
118-
- central-db
119116
- stackrox-operator-bundle
120117
- stackrox-operator-index
121118

@@ -136,7 +133,7 @@ make image
136133

137134
# Deploy to kind - roxie automatically uses local images
138135
cd /path/to/roxie
139-
./roxie deploy both
136+
./roxie deploy
140137
```
141138

142139
### Behavior

internal/cluster/kind.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ func isKindContext(contextName string) bool {
2121
return strings.HasPrefix(strings.ToLower(contextName), "kind")
2222
}
2323

24-
// ExtractKindClusterName extracts the cluster name from a kind context.
24+
// extractKindClusterName extracts the cluster name from a kind context.
2525
// For context "kind-acs", returns "acs".
2626
// For context "kind", returns "kind".
27-
func ExtractKindClusterName(contextName string) string {
27+
func extractKindClusterName(contextName string) string {
2828
// Remove "kind-" prefix if present
2929
if strings.HasPrefix(strings.ToLower(contextName), "kind-") {
3030
return contextName[len("kind-"):]
@@ -39,5 +39,5 @@ func GetKindClusterName() string {
3939
if !IsKindCluster() {
4040
return ""
4141
}
42-
return ExtractKindClusterName(env.GetCurrentContext())
42+
return extractKindClusterName(env.GetCurrentContext())
4343
}

internal/cluster/kind_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ func TestExtractKindClusterName(t *testing.T) {
8282

8383
for _, tt := range tests {
8484
t.Run(tt.name, func(t *testing.T) {
85-
result := ExtractKindClusterName(tt.contextName)
85+
result := extractKindClusterName(tt.contextName)
8686
if result != tt.expected {
87-
t.Errorf("ExtractKindClusterName(%q) = %v, want %v", tt.contextName, result, tt.expected)
87+
t.Errorf("extractKindClusterName(%q) = %v, want %v", tt.contextName, result, tt.expected)
8888
}
8989
})
9090
}

internal/deployer/deploy_via_operator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func (d *Deployer) prepareNamespace(ctx context.Context, namespace string) error
180180
}
181181

182182
if env.GetCurrentClusterType() != env.InfraOpenShift4 {
183-
if !d.shouldSkipImagePullSecrets() {
183+
if !d.hasAllImagesLocally() {
184184
if err := d.ensurePullSecretExists(ctx, namespace); err != nil {
185185
return fmt.Errorf("ensuring image pull secret exists: %w", err)
186186
}

internal/deployer/deployer.go

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ const (
2727
// totalRequiredImages is the number of images needed for a complete deployment
2828
// (7 main images + 2 operator images: stackrox-operator + stackrox-operator-bundle).
2929
// Main images: main, scanner, scanner-db, scanner-v4, scanner-v4-db, central-db, collector
30-
// Note: scanner-v4 is a single image that runs in different modes (indexer/matcher)
31-
// based on runtime configuration.
32-
// Note: operator-index is not included as roxie doesn't use it in default (non-OLM) mode.
3330
totalRequiredImages = 9
3431

3532
// skipLocalImagesEnvVar is the environment variable name to disable local image detection.
@@ -134,9 +131,8 @@ type Deployer struct {
134131
earlyReadiness bool
135132
dockerCreds *dockerauth.Credentials
136133
clusterResourceKinds map[string]struct{}
137-
// New fields for local image support
138-
localImages map[string]string // map of image names to local references
139-
usingLocalImages bool // true if any local images were found and loaded
134+
localImages map[string]string // map of image names to local references
135+
usingLocalImages bool
140136
}
141137

142138
type ResourceKindWithName struct {
@@ -445,7 +441,7 @@ func (d *Deployer) Deploy(ctx context.Context, component, resources, exposure st
445441

446442
// Prepare and verify credentials early to fail fast
447443
// Skip if all images are local
448-
if !d.shouldSkipCredentialVerification() {
444+
if !d.hasAllImagesLocally() {
449445
if err := d.prepareCredentials(); err != nil {
450446
return fmt.Errorf("failed to prepare credentials: %w", err)
451447
}
@@ -491,19 +487,14 @@ func (d *Deployer) prepareCredentials() error {
491487
}
492488

493489
// detectAndLoadLocalImages attempts to detect and load locally-available container
494-
// images into the kind cluster. It performs the following steps:
495-
// 1. Checks if ROXIE_SKIP_LOCAL_IMAGES environment variable is set
496-
// 2. Verifies the target cluster is a kind cluster
497-
// 3. Queries podman for locally available images
498-
// 4. Loads discovered images into the kind cluster
490+
// images into the kind cluster.
499491
//
500492
// This method gracefully degrades - if podman is unavailable or no images are found,
501493
// it returns nil and allows deployment to proceed with remote image pulls.
502494
// Returns an error only if image loading into kind fails after images are detected.
503495
func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
504-
// Check if ROXIE_SKIP_LOCAL_IMAGES is set
505496
if os.Getenv(skipLocalImagesEnvVar) == "true" {
506-
d.logger.Dim("ROXIE_SKIP_LOCAL_IMAGES is set, skipping local image detection")
497+
d.logger.Dim(skipLocalImagesEnvVar + " is set, skipping local image detection")
507498
return nil
508499
}
509500

@@ -520,7 +511,6 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
520511
d.logger.Dim("Checking for local images in podman...")
521512
localImages, err := localimages.CheckImages(d.mainImageTag, d.operatorTag)
522513
if err != nil {
523-
// If podman is not available, gracefully fall back
524514
d.logger.Dimf("Could not check for local images: %v", err)
525515
d.logger.Dim("Falling back to quay.io")
526516
return nil
@@ -531,13 +521,11 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
531521
return nil
532522
}
533523

534-
// Calculate total images needed (7 main + 2 operator = 9)
535-
totalExpected := totalRequiredImages
536-
d.logger.Infof("Found %d/%d images locally in podman", len(localImages), totalExpected)
524+
d.logger.Infof("Found %d/%d images locally in podman", len(localImages), totalRequiredImages)
537525

538526
// Load images into kind using quay.io paths
539527
if err := localimages.LoadImagesToKind(ctx, localImages, d.mainImageTag, d.operatorTag, kindClusterName, d.logger); err != nil {
540-
return fmt.Errorf("failed to load %d images into kind cluster %s: %w", len(localImages), kindClusterName, err)
528+
return fmt.Errorf("failed to load images into kind cluster %s: %w", kindClusterName, err)
541529
}
542530

543531
// Store the local images for later use
@@ -547,34 +535,26 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
547535
return nil
548536
}
549537

550-
// shouldSkipCredentialVerification returns true if credential verification should be skipped.
551-
// Verification is skipped only when all required images (9 total) are available locally.
552-
// For partial local image scenarios, verification is still required for remote pulls.
553-
func (d *Deployer) shouldSkipCredentialVerification() bool {
554-
// If not using any local images, don't skip
538+
// hasAllImagesLocally returns true if all required images are available locally.
539+
// When all images are local, credential verification and image pull secrets can be skipped.
540+
// For partial local image scenarios, credentials are still required for remote pulls.
541+
func (d *Deployer) hasAllImagesLocally() bool {
542+
// If not using any local images, return false
555543
if !d.usingLocalImages {
556544
return false
557545
}
558546

559-
// If using some local images but not all, don't skip (need creds for remote pulls)
560-
// Total expected: 7 main + 2 operator = 9
561-
totalExpected := totalRequiredImages
562-
if len(d.localImages) < totalExpected {
547+
// If using some local images but not all, return false (need creds for remote pulls)
548+
if len(d.localImages) < totalRequiredImages {
563549
d.logger.Dimf("Using %d/%d local images, remaining images will be pulled from quay.io",
564-
len(d.localImages), totalExpected)
550+
len(d.localImages), totalRequiredImages)
565551
return false
566552
}
567553

568554
// All images are local
569555
return true
570556
}
571557

572-
// shouldSkipImagePullSecrets returns true if image pull secrets should be skipped.
573-
// Same logic as credential verification - skip only if all images are local.
574-
func (d *Deployer) shouldSkipImagePullSecrets() bool {
575-
return d.shouldSkipCredentialVerification()
576-
}
577-
578558
func (d *Deployer) deployCentral(ctx context.Context, resources, exposure string) error {
579559
d.logger.Infof("Deploying Central to namespace %s", d.centralNamespace)
580560
if d.namespaceExists(d.centralNamespace) {

internal/deployer/operator.go

Lines changed: 55 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,43 @@ const (
2525
operatorDeploymentName = "rhacs-operator-controller-manager"
2626
)
2727

28+
// clusterServiceVersion represents the relevant structure of a ClusterServiceVersion YAML
29+
// that we need to patch for local image references. Uses inline maps to preserve
30+
// all fields not explicitly defined.
31+
type clusterServiceVersion struct {
32+
Spec struct {
33+
Install struct {
34+
Spec struct {
35+
Deployments []struct {
36+
Spec struct {
37+
Template struct {
38+
Spec struct {
39+
Containers []struct {
40+
Image string `yaml:"image"`
41+
Env []struct {
42+
Name string `yaml:"name"`
43+
Value string `yaml:"value"`
44+
ExtraFields map[string]interface{} `yaml:",inline"`
45+
} `yaml:"env"`
46+
ExtraFields map[string]interface{} `yaml:",inline"`
47+
} `yaml:"containers"`
48+
ExtraFields map[string]interface{} `yaml:",inline"`
49+
} `yaml:"spec"`
50+
ExtraFields map[string]interface{} `yaml:",inline"`
51+
} `yaml:"template"`
52+
ExtraFields map[string]interface{} `yaml:",inline"`
53+
} `yaml:"spec"`
54+
ExtraFields map[string]interface{} `yaml:",inline"`
55+
} `yaml:"deployments"`
56+
ExtraFields map[string]interface{} `yaml:",inline"`
57+
} `yaml:"spec"`
58+
ExtraFields map[string]interface{} `yaml:",inline"`
59+
} `yaml:"install"`
60+
ExtraFields map[string]interface{} `yaml:",inline"`
61+
} `yaml:"spec"`
62+
ExtraFields map[string]interface{} `yaml:",inline"`
63+
}
64+
2865
// deployOperator deploys the RHACS operator
2966
func (d *Deployer) deployOperator(ctx context.Context) error {
3067
d.logger.Infof("Operator tag: %s", d.operatorTag)
@@ -331,7 +368,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string)
331368
// Patch CSV with local image references if using local images
332369
if d.usingLocalImages {
333370
d.logger.Info("Patching CSV with local image references")
334-
if err := patchCSVWithLocalImages(csvFile, d.mainImageTag, d.localImages); err != nil {
371+
if err := patchCSVWithLocalImages(csvFile, d.mainImageTag, d.operatorTag, d.localImages); err != nil {
335372
return fmt.Errorf("failed to patch CSV with local images: %w", err)
336373
}
337374
}
@@ -383,9 +420,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string)
383420
// envVarToImageName converts a RELATED_IMAGE_* environment variable name to an image name.
384421
// e.g., RELATED_IMAGE_SCANNER_V4_DB → scanner-v4-db
385422
func envVarToImageName(envVar string) string {
386-
// Remove RELATED_IMAGE_ prefix
387423
name := strings.TrimPrefix(envVar, "RELATED_IMAGE_")
388-
// Convert to lowercase and replace underscores with hyphens
389424
name = strings.ToLower(name)
390425
name = strings.ReplaceAll(name, "_", "-")
391426
return name
@@ -398,86 +433,34 @@ func envVarToImageName(envVar string) string {
398433
//
399434
// The function only patches images that exist in the localImages map, leaving
400435
// other image references unchanged.
401-
func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[string]string) error {
402-
// Return early if no local images to patch
403-
if len(localImages) == 0 {
404-
return nil
405-
}
406-
436+
func patchCSVWithLocalImages(csvFile, mainImageTag, operatorTag string, localImages map[string]string) error {
407437
// Read the CSV file
408438
content, err := os.ReadFile(csvFile)
409439
if err != nil {
410440
return fmt.Errorf("failed to read CSV file: %w", err)
411441
}
412442

413-
// Unmarshal to map[string]interface{}
414-
var csvData map[string]interface{}
443+
var csvData clusterServiceVersion
415444
if err := yaml.Unmarshal(content, &csvData); err != nil {
416445
return fmt.Errorf("failed to parse CSV YAML: %w", err)
417446
}
418447

419-
// Navigate to the container spec
420-
spec, ok := csvData["spec"].(map[string]interface{})
421-
if !ok {
422-
return errors.New("CSV missing 'spec' field")
423-
}
424-
425-
install, ok := spec["install"].(map[string]interface{})
426-
if !ok {
427-
return errors.New("CSV missing 'spec.install' field")
428-
}
429-
430-
installSpec, ok := install["spec"].(map[string]interface{})
431-
if !ok {
432-
return errors.New("CSV missing 'spec.install.spec' field")
433-
}
434-
435-
deployments, ok := installSpec["deployments"].([]interface{})
436-
if !ok || len(deployments) == 0 {
437-
return errors.New("CSV missing 'spec.install.spec.deployments' field or deployments array is empty")
438-
}
439-
440-
deployment, ok := deployments[0].(map[string]interface{})
441-
if !ok {
442-
return errors.New("invalid deployment structure in CSV")
443-
}
444-
445-
deploymentSpec, ok := deployment["spec"].(map[string]interface{})
446-
if !ok {
447-
return errors.New("CSV missing deployment spec")
448+
// Validate structure
449+
if len(csvData.Spec.Install.Spec.Deployments) == 0 {
450+
return errors.New("CSV missing deployments")
448451
}
449-
450-
template, ok := deploymentSpec["template"].(map[string]interface{})
451-
if !ok {
452-
return errors.New("CSV missing deployment template")
453-
}
454-
455-
podSpec, ok := template["spec"].(map[string]interface{})
456-
if !ok {
457-
return errors.New("CSV missing pod spec")
458-
}
459-
460-
containers, ok := podSpec["containers"].([]interface{})
461-
if !ok || len(containers) == 0 {
462-
return errors.New("CSV missing containers or containers array is empty")
452+
if len(csvData.Spec.Install.Spec.Deployments[0].Spec.Template.Spec.Containers) == 0 {
453+
return errors.New("CSV missing containers")
463454
}
464455

465-
container, ok := containers[0].(map[string]interface{})
466-
if !ok {
467-
return errors.New("invalid container structure in CSV")
468-
}
456+
// Get reference to the first container
457+
container := &csvData.Spec.Install.Spec.Deployments[0].Spec.Template.Spec.Containers[0]
469458

470459
// Patch operator image if it exists in localImages
471460
// Use the actual detected image reference to handle fallback branding cases
472-
operatorImageKey := "stackrox-operator:" + mainImageTag
461+
operatorImageKey := "stackrox-operator:" + operatorTag
473462
if imageRef, ok := localImages[operatorImageKey]; ok {
474-
container["image"] = imageRef
475-
}
476-
477-
// Patch RELATED_IMAGE_* environment variables
478-
envVars, ok := container["env"].([]interface{})
479-
if !ok {
480-
return errors.New("CSV missing container env variables")
463+
container.Image = imageRef
481464
}
482465

483466
// Build a reverse map from image name to full image reference for quick lookup
@@ -491,25 +474,18 @@ func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[strin
491474
}
492475
}
493476

494-
for _, envVar := range envVars {
495-
envMap, ok := envVar.(map[string]interface{})
496-
if !ok {
497-
continue
498-
}
499-
500-
envName, ok := envMap["name"].(string)
501-
if !ok {
502-
continue
503-
}
477+
// Patch RELATED_IMAGE_* environment variables
478+
for i := range container.Env {
479+
envVar := &container.Env[i]
504480

505481
// Check if this is a RELATED_IMAGE_* env var
506-
if !strings.HasPrefix(envName, "RELATED_IMAGE_") {
482+
if !strings.HasPrefix(envVar.Name, "RELATED_IMAGE_") {
507483
continue
508484
}
509485

510486
// Convert RELATED_IMAGE_FOO_BAR to foo-bar
511487
// e.g., RELATED_IMAGE_SCANNER_V4_DB → scanner-v4-db
512-
imageName := envVarToImageName(envName)
488+
imageName := envVarToImageName(envVar.Name)
513489

514490
// Special case: scanner-v4-indexer and scanner-v4-matcher both use the scanner-v4 image
515491
// The same image runs in different modes based on runtime configuration
@@ -520,7 +496,7 @@ func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[strin
520496
// Check if we have this image locally and use the actual detected reference
521497
// This handles cases where an image was found at a fallback branding org
522498
if imageRef, found := imageNameToRef[imageName]; found {
523-
envMap["value"] = imageRef
499+
envVar.Value = imageRef
524500
}
525501
}
526502

0 commit comments

Comments
 (0)