Skip to content

Commit 9d8a54b

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 a0ca137 commit 9d8a54b

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
@@ -167,7 +167,7 @@ func (d *Deployer) prepareNamespace(ctx context.Context, namespace string) error
167167
}
168168

169169
if env.GetCurrentClusterType() != env.InfraOpenShift4 {
170-
if !d.shouldSkipImagePullSecrets() {
170+
if !d.hasAllImagesLocally() {
171171
if err := d.ensurePullSecretExists(ctx, namespace); err != nil {
172172
return fmt.Errorf("ensuring image pull secret exists: %w", err)
173173
}

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.
@@ -132,9 +129,8 @@ type Deployer struct {
132129
earlyReadiness bool
133130
dockerCreds *dockerauth.Credentials
134131
clusterResourceKinds map[string]struct{}
135-
// New fields for local image support
136-
localImages map[string]string // map of image names to local references
137-
usingLocalImages bool // true if any local images were found and loaded
132+
localImages map[string]string // map of image names to local references
133+
usingLocalImages bool
138134
}
139135

140136
type ResourceKindWithName struct {
@@ -406,7 +402,7 @@ func (d *Deployer) Deploy(ctx context.Context, component, resources, exposure st
406402

407403
// Prepare and verify credentials early to fail fast
408404
// Skip if all images are local
409-
if !d.shouldSkipCredentialVerification() {
405+
if !d.hasAllImagesLocally() {
410406
if err := d.prepareCredentials(); err != nil {
411407
return fmt.Errorf("failed to prepare credentials: %w", err)
412408
}
@@ -450,19 +446,14 @@ func (d *Deployer) prepareCredentials() error {
450446
}
451447

452448
// detectAndLoadLocalImages attempts to detect and load locally-available container
453-
// images into the kind cluster. It performs the following steps:
454-
// 1. Checks if ROXIE_SKIP_LOCAL_IMAGES environment variable is set
455-
// 2. Verifies the target cluster is a kind cluster
456-
// 3. Queries podman for locally available images
457-
// 4. Loads discovered images into the kind cluster
449+
// images into the kind cluster.
458450
//
459451
// This method gracefully degrades - if podman is unavailable or no images are found,
460452
// it returns nil and allows deployment to proceed with remote image pulls.
461453
// Returns an error only if image loading into kind fails after images are detected.
462454
func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
463-
// Check if ROXIE_SKIP_LOCAL_IMAGES is set
464455
if os.Getenv(skipLocalImagesEnvVar) == "true" {
465-
d.logger.Dim("ROXIE_SKIP_LOCAL_IMAGES is set, skipping local image detection")
456+
d.logger.Dim(skipLocalImagesEnvVar + " is set, skipping local image detection")
466457
return nil
467458
}
468459

@@ -479,7 +470,6 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
479470
d.logger.Dim("Checking for local images in podman...")
480471
localImages, err := localimages.CheckImages(d.mainImageTag, d.operatorTag)
481472
if err != nil {
482-
// If podman is not available, gracefully fall back
483473
d.logger.Dimf("Could not check for local images: %v", err)
484474
d.logger.Dim("Falling back to quay.io")
485475
return nil
@@ -490,13 +480,11 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
490480
return nil
491481
}
492482

493-
// Calculate total images needed (7 main + 2 operator = 9)
494-
totalExpected := totalRequiredImages
495-
d.logger.Infof("Found %d/%d images locally in podman", len(localImages), totalExpected)
483+
d.logger.Infof("Found %d/%d images locally in podman", len(localImages), totalRequiredImages)
496484

497485
// Load images into kind using quay.io paths
498486
if err := localimages.LoadImagesToKind(ctx, localImages, d.mainImageTag, d.operatorTag, kindClusterName, d.logger); err != nil {
499-
return fmt.Errorf("failed to load %d images into kind cluster %s: %w", len(localImages), kindClusterName, err)
487+
return fmt.Errorf("failed to load images into kind cluster %s: %w", kindClusterName, err)
500488
}
501489

502490
// Store the local images for later use
@@ -506,34 +494,26 @@ func (d *Deployer) detectAndLoadLocalImages(ctx context.Context) error {
506494
return nil
507495
}
508496

509-
// shouldSkipCredentialVerification returns true if credential verification should be skipped.
510-
// Verification is skipped only when all required images (9 total) are available locally.
511-
// For partial local image scenarios, verification is still required for remote pulls.
512-
func (d *Deployer) shouldSkipCredentialVerification() bool {
513-
// If not using any local images, don't skip
497+
// hasAllImagesLocally returns true if all required images are available locally.
498+
// When all images are local, credential verification and image pull secrets can be skipped.
499+
// For partial local image scenarios, credentials are still required for remote pulls.
500+
func (d *Deployer) hasAllImagesLocally() bool {
501+
// If not using any local images, return false
514502
if !d.usingLocalImages {
515503
return false
516504
}
517505

518-
// If using some local images but not all, don't skip (need creds for remote pulls)
519-
// Total expected: 7 main + 2 operator = 9
520-
totalExpected := totalRequiredImages
521-
if len(d.localImages) < totalExpected {
506+
// If using some local images but not all, return false (need creds for remote pulls)
507+
if len(d.localImages) < totalRequiredImages {
522508
d.logger.Dimf("Using %d/%d local images, remaining images will be pulled from quay.io",
523-
len(d.localImages), totalExpected)
509+
len(d.localImages), totalRequiredImages)
524510
return false
525511
}
526512

527513
// All images are local
528514
return true
529515
}
530516

531-
// shouldSkipImagePullSecrets returns true if image pull secrets should be skipped.
532-
// Same logic as credential verification - skip only if all images are local.
533-
func (d *Deployer) shouldSkipImagePullSecrets() bool {
534-
return d.shouldSkipCredentialVerification()
535-
}
536-
537517
func (d *Deployer) deployCentral(ctx context.Context, resources, exposure string) error {
538518
d.logger.Infof("Deploying Central to namespace %s", d.centralNamespace)
539519
if d.namespaceExists(d.centralNamespace) {

internal/deployer/operator.go

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

27+
// clusterServiceVersion represents the relevant structure of a ClusterServiceVersion YAML
28+
// that we need to patch for local image references. Uses inline maps to preserve
29+
// all fields not explicitly defined.
30+
type clusterServiceVersion struct {
31+
Spec struct {
32+
Install struct {
33+
Spec struct {
34+
Deployments []struct {
35+
Spec struct {
36+
Template struct {
37+
Spec struct {
38+
Containers []struct {
39+
Image string `yaml:"image"`
40+
Env []struct {
41+
Name string `yaml:"name"`
42+
Value string `yaml:"value"`
43+
ExtraFields map[string]interface{} `yaml:",inline"`
44+
} `yaml:"env"`
45+
ExtraFields map[string]interface{} `yaml:",inline"`
46+
} `yaml:"containers"`
47+
ExtraFields map[string]interface{} `yaml:",inline"`
48+
} `yaml:"spec"`
49+
ExtraFields map[string]interface{} `yaml:",inline"`
50+
} `yaml:"template"`
51+
ExtraFields map[string]interface{} `yaml:",inline"`
52+
} `yaml:"spec"`
53+
ExtraFields map[string]interface{} `yaml:",inline"`
54+
} `yaml:"deployments"`
55+
ExtraFields map[string]interface{} `yaml:",inline"`
56+
} `yaml:"spec"`
57+
ExtraFields map[string]interface{} `yaml:",inline"`
58+
} `yaml:"install"`
59+
ExtraFields map[string]interface{} `yaml:",inline"`
60+
} `yaml:"spec"`
61+
ExtraFields map[string]interface{} `yaml:",inline"`
62+
}
63+
2764
// deployOperator deploys the RHACS operator
2865
func (d *Deployer) deployOperator(ctx context.Context) error {
2966
d.logger.Infof("Operator tag: %s", d.operatorTag)
@@ -223,7 +260,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string)
223260
// Patch CSV with local image references if using local images
224261
if d.usingLocalImages {
225262
d.logger.Info("Patching CSV with local image references")
226-
if err := patchCSVWithLocalImages(csvFile, d.mainImageTag, d.localImages); err != nil {
263+
if err := patchCSVWithLocalImages(csvFile, d.mainImageTag, d.operatorTag, d.localImages); err != nil {
227264
return fmt.Errorf("failed to patch CSV with local images: %w", err)
228265
}
229266
}
@@ -275,9 +312,7 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string)
275312
// envVarToImageName converts a RELATED_IMAGE_* environment variable name to an image name.
276313
// e.g., RELATED_IMAGE_SCANNER_V4_DB → scanner-v4-db
277314
func envVarToImageName(envVar string) string {
278-
// Remove RELATED_IMAGE_ prefix
279315
name := strings.TrimPrefix(envVar, "RELATED_IMAGE_")
280-
// Convert to lowercase and replace underscores with hyphens
281316
name = strings.ToLower(name)
282317
name = strings.ReplaceAll(name, "_", "-")
283318
return name
@@ -290,86 +325,34 @@ func envVarToImageName(envVar string) string {
290325
//
291326
// The function only patches images that exist in the localImages map, leaving
292327
// other image references unchanged.
293-
func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[string]string) error {
294-
// Return early if no local images to patch
295-
if len(localImages) == 0 {
296-
return nil
297-
}
298-
328+
func patchCSVWithLocalImages(csvFile, mainImageTag, operatorTag string, localImages map[string]string) error {
299329
// Read the CSV file
300330
content, err := os.ReadFile(csvFile)
301331
if err != nil {
302332
return fmt.Errorf("failed to read CSV file: %w", err)
303333
}
304334

305-
// Unmarshal to map[string]interface{}
306-
var csvData map[string]interface{}
335+
var csvData clusterServiceVersion
307336
if err := yaml.Unmarshal(content, &csvData); err != nil {
308337
return fmt.Errorf("failed to parse CSV YAML: %w", err)
309338
}
310339

311-
// Navigate to the container spec
312-
spec, ok := csvData["spec"].(map[string]interface{})
313-
if !ok {
314-
return errors.New("CSV missing 'spec' field")
315-
}
316-
317-
install, ok := spec["install"].(map[string]interface{})
318-
if !ok {
319-
return errors.New("CSV missing 'spec.install' field")
320-
}
321-
322-
installSpec, ok := install["spec"].(map[string]interface{})
323-
if !ok {
324-
return errors.New("CSV missing 'spec.install.spec' field")
325-
}
326-
327-
deployments, ok := installSpec["deployments"].([]interface{})
328-
if !ok || len(deployments) == 0 {
329-
return errors.New("CSV missing 'spec.install.spec.deployments' field or deployments array is empty")
330-
}
331-
332-
deployment, ok := deployments[0].(map[string]interface{})
333-
if !ok {
334-
return errors.New("invalid deployment structure in CSV")
335-
}
336-
337-
deploymentSpec, ok := deployment["spec"].(map[string]interface{})
338-
if !ok {
339-
return errors.New("CSV missing deployment spec")
340+
// Validate structure
341+
if len(csvData.Spec.Install.Spec.Deployments) == 0 {
342+
return errors.New("CSV missing deployments")
340343
}
341-
342-
template, ok := deploymentSpec["template"].(map[string]interface{})
343-
if !ok {
344-
return errors.New("CSV missing deployment template")
345-
}
346-
347-
podSpec, ok := template["spec"].(map[string]interface{})
348-
if !ok {
349-
return errors.New("CSV missing pod spec")
350-
}
351-
352-
containers, ok := podSpec["containers"].([]interface{})
353-
if !ok || len(containers) == 0 {
354-
return errors.New("CSV missing containers or containers array is empty")
344+
if len(csvData.Spec.Install.Spec.Deployments[0].Spec.Template.Spec.Containers) == 0 {
345+
return errors.New("CSV missing containers")
355346
}
356347

357-
container, ok := containers[0].(map[string]interface{})
358-
if !ok {
359-
return errors.New("invalid container structure in CSV")
360-
}
348+
// Get reference to the first container
349+
container := &csvData.Spec.Install.Spec.Deployments[0].Spec.Template.Spec.Containers[0]
361350

362351
// Patch operator image if it exists in localImages
363352
// Use the actual detected image reference to handle fallback branding cases
364-
operatorImageKey := "stackrox-operator:" + mainImageTag
353+
operatorImageKey := "stackrox-operator:" + operatorTag
365354
if imageRef, ok := localImages[operatorImageKey]; ok {
366-
container["image"] = imageRef
367-
}
368-
369-
// Patch RELATED_IMAGE_* environment variables
370-
envVars, ok := container["env"].([]interface{})
371-
if !ok {
372-
return errors.New("CSV missing container env variables")
355+
container.Image = imageRef
373356
}
374357

375358
// Build a reverse map from image name to full image reference for quick lookup
@@ -383,25 +366,18 @@ func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[strin
383366
}
384367
}
385368

386-
for _, envVar := range envVars {
387-
envMap, ok := envVar.(map[string]interface{})
388-
if !ok {
389-
continue
390-
}
391-
392-
envName, ok := envMap["name"].(string)
393-
if !ok {
394-
continue
395-
}
369+
// Patch RELATED_IMAGE_* environment variables
370+
for i := range container.Env {
371+
envVar := &container.Env[i]
396372

397373
// Check if this is a RELATED_IMAGE_* env var
398-
if !strings.HasPrefix(envName, "RELATED_IMAGE_") {
374+
if !strings.HasPrefix(envVar.Name, "RELATED_IMAGE_") {
399375
continue
400376
}
401377

402378
// Convert RELATED_IMAGE_FOO_BAR to foo-bar
403379
// e.g., RELATED_IMAGE_SCANNER_V4_DB → scanner-v4-db
404-
imageName := envVarToImageName(envName)
380+
imageName := envVarToImageName(envVar.Name)
405381

406382
// Special case: scanner-v4-indexer and scanner-v4-matcher both use the scanner-v4 image
407383
// The same image runs in different modes based on runtime configuration
@@ -412,7 +388,7 @@ func patchCSVWithLocalImages(csvFile, mainImageTag string, localImages map[strin
412388
// Check if we have this image locally and use the actual detected reference
413389
// This handles cases where an image was found at a fallback branding org
414390
if imageRef, found := imageNameToRef[imageName]; found {
415-
envMap["value"] = imageRef
391+
envVar.Value = imageRef
416392
}
417393
}
418394

0 commit comments

Comments
 (0)