Merge branch 'main' into backup/mc/new-config-2

Author: Moritz Clasmeier

cmd/deploy.go

@@ -534,7 +534,7 @@ func deployValidate(components component.Component, deploySettings *deployer.Con
 			return errors.New("using Konflux images while deploying operator via OLM is not supported")
 		}
 		clusterType := env.GetCurrentClusterType()
-		if clusterType != types.ClusterTypeInfraOpenShift4 {
+		if !clusterType.IsOpenShift() {
 			return fmt.Errorf("--konflux flag is only supported on OpenShift 4 clusters (current cluster type: %s)", clusterType.String())
 		}
 	}

internal/deployer/deploy_via_operator.go

@@ -56,7 +56,10 @@ func (d *Deployer) ensureOperatorDeployed(ctx context.Context) error {
 	}
 
 	// Detect current operator deployment mode
-	operatorExists, currentMode := d.detectOperatorDeploymentMode(ctx)
+	operatorExists, currentMode, err := d.detectOperatorDeploymentMode(ctx)
+	if err != nil {
+		return fmt.Errorf("detecting operator deployment mode: %w", err)
+	}
 	needsDeployment := false
 	needsTeardown := false
 

internal/deployer/operator.go

@@ -17,7 +17,6 @@ import (
 	"github.com/stackrox/roxie/internal/env"
 	"github.com/stackrox/roxie/internal/k8s"
 	"github.com/stackrox/roxie/internal/ocihelper"
-	"github.com/stackrox/roxie/internal/types"
 )
 
 const (
@@ -202,7 +201,7 @@ func (d *Deployer) getOperatorBundleImage() string {
 
 // ensureKonfluxImageRewriting configures image rewriting for Konflux images
 func (d *Deployer) ensureKonfluxImageRewriting(ctx context.Context) error {
-	if env.GetCurrentClusterType() != types.ClusterTypeInfraOpenShift4 {
+	if !env.GetCurrentClusterType().IsOpenShift() {
 		return errors.New("image rewriting for Konflux is only supported on OpenShift4 clusters")
 	}
 
@@ -290,7 +289,7 @@ func (d *Deployer) applyImageContentSourcePolicy(ctx context.Context) error {
 
 // removeKonfluxImageRewriting removes the ImageContentSourcePolicy for Konflux images if it exists
 func (d *Deployer) removeKonfluxImageRewriting(ctx context.Context) error {
-	if env.GetCurrentClusterType() != types.ClusterTypeInfraOpenShift4 {
+	if !env.GetCurrentClusterType().IsOpenShift() {
 		return nil
 	}
 
@@ -643,7 +642,10 @@ func (d *Deployer) teardownOperatorNonOLM(ctx context.Context) error {
 
 // teardownOperator removes the operator if it exists, detecting the deployment mode automatically.
 func (d *Deployer) teardownOperator(ctx context.Context) error {
-	operatorExists, operatorMode := d.detectOperatorDeploymentMode(ctx)
+	operatorExists, operatorMode, err := d.detectOperatorDeploymentMode(ctx)
+	if err != nil {
+		return fmt.Errorf("detecting operator deployment mode: %w", err)
+	}
 	if !operatorExists {
 		d.logger.Dim("No operator deployment found, skipping operator teardown")
 		return nil

internal/deployer/operator_olm.go

@@ -335,35 +335,34 @@ func (d *Deployer) waitForCSVSuccess(ctx context.Context) error {
 
 // detectOperatorDeploymentMode detects how the operator is currently deployed.
 // Returns (operatorExists bool, isOLM OperatorDeploymentMode)
-func (d *Deployer) detectOperatorDeploymentMode(ctx context.Context) (bool, OperatorDeploymentMode) {
+func (d *Deployer) detectOperatorDeploymentMode(ctx context.Context) (bool, OperatorDeploymentMode, error) {
+	const olmOwnerLabel = "olm.owner"
+
 	// First, check if a Subscription exists (OLM-specific resource)
 	_, err := d.runKubectl(ctx, k8s.KubectlOptions{
 		Args: []string{"get", "subscription", subscriptionName, "-n", operatorNamespace},
 	})
 	if err == nil {
-		return true, OperatorModeOLM
+		return true, OperatorModeOLM, nil
 	}
 
-	// If no subscription, check if operator deployment exists.
-	_, err = d.runKubectl(ctx, k8s.KubectlOptions{
-		Args: []string{"get", "deployment", operatorDeploymentName, "-n", operatorNamespace},
-	})
-	if err == nil {
-		// Deployment exists - check if it has OLM owner labels.
-		result, err := d.runKubectl(ctx, k8s.KubectlOptions{
-			Args: []string{"get", "deployment", operatorDeploymentName, "-n", operatorNamespace, "-o", "jsonpath={.metadata.labels}"},
-		})
-		// TODO(ROX-34499): This is not very robust. Better retrieve a specific label in the `get`
-		// command?
-		if err == nil && strings.Contains(result.Stdout, "olm.owner") {
-			return true, OperatorModeOLM
-		}
-		// Deployment exists without OLM labels = non-OLM deployment.
-		return true, OperatorModeNonOLM
+	// If no subscription, check if operator deployment exists/if it has the expected OLM label.
+	labelValue, err := k8s.RetrieveClusterResourceLabel(ctx, d.logger, operatorNamespace, "deployment", operatorDeploymentName, olmOwnerLabel)
+	if k8s.IsResourceNotFound(err) {
+		// No operator deployment found.
+		return false, OperatorModeNonOLM, nil
+	}
+	if err != nil {
+		return false, OperatorModeNonOLM, err
+	}
+
+	if labelValue == "" {
+		// Deployment exists without OLM labels -> non-OLM deployment.
+		return true, OperatorModeNonOLM, nil
 	}
 
-	// No operator found.
-	return false, OperatorModeNonOLM
+	// Label set -> OLM deployment.
+	return true, OperatorModeOLM, nil
 }
 
 // teardownOperatorOLM removes the operator when installed via OLM.

internal/env/env.go

@@ -147,22 +147,12 @@ func DetectClusterType(config KubeConfig, apiResources []string) types.ClusterTy
 		return types.ClusterTypeInfraGKE
 	}
 
-	// Minikube clusters typically have context name "minikube".
-	if contextLower == "minikube" || strings.HasPrefix(contextLower, "minikube-") {
-		return types.ClusterTypeMinikube
-	}
-
-	// Check for OpenShift 4 clusters by examining the server hostname.
-	if serverURL := getServerURL(config); serverURL != "" {
-		if parsedURL, err := url.Parse(serverURL); err == nil {
-			hostname := parsedURL.Hostname()
-			if strings.HasSuffix(hostname, ".ocp.infra.rox.systems") {
-				// Further verify it's OpenShift 4 by checking the API resources
-				if isOpenShift4(apiResources) {
-					return types.ClusterTypeInfraOpenShift4
-				}
-			}
+	// Check for OpenShift 4 clusters.
+	if isOpenShift4(apiResources) {
+		if isInfraOpenShift4(config) {
+			return types.ClusterTypeInfraOpenShift4
 		}
+		return types.ClusterTypeOpenShift4
 	}
 
 	// K3s clusters often have "k3s" in the context name
@@ -184,6 +174,19 @@ func DetectClusterType(config KubeConfig, apiResources []string) types.ClusterTy
 	return types.ClusterTypeUnknown
 }
 
+func isInfraOpenShift4(config KubeConfig) bool {
+	serverURL := getServerURL(config)
+	if serverURL == "" {
+		return false
+	}
+	parsedURL, err := url.Parse(serverURL)
+	if err != nil {
+		return false
+	}
+	hostname := parsedURL.Hostname()
+	return strings.HasSuffix(hostname, ".ocp.infra.rox.systems")
+}
+
 // getServerURL retrieves the server URL from the KubeConfig
 func getServerURL(config KubeConfig) string {
 	if len(config.Clusters) == 0 {

internal/env/env_test.go

@@ -3,6 +3,8 @@ package env
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/stackrox/roxie/internal/types"
 )
 
@@ -42,7 +44,7 @@ func TestDetectClusterType_GKE_ExactMatch(t *testing.T) {
 	}
 }
 
-func TestDetectClusterType_OpenShift4(t *testing.T) {
+func TestDetectClusterType_InfraOpenShift4(t *testing.T) {
 	config := KubeConfig{
 		CurrentContext: "admin",
 		Clusters: []KubeCluster{
@@ -60,31 +62,28 @@ func TestDetectClusterType_OpenShift4(t *testing.T) {
 	}
 
 	result := DetectClusterType(config, apiResources)
-	if result != types.ClusterTypeInfraOpenShift4 {
-		t.Errorf("DetectClusterType() = %v (%s), want %v", result, result.String(), types.ClusterTypeInfraOpenShift4)
-	}
+	assert.Equal(t, types.ClusterTypeInfraOpenShift4, result)
 }
 
-func TestDetectClusterType_OpenShift4_WrongHostname(t *testing.T) {
+func TestDetectClusterType_OpenShift4(t *testing.T) {
 	config := KubeConfig{
-		CurrentContext: "admin",
+		CurrentContext: "some-context-name",
 		Clusters: []KubeCluster{
 			{
-				Name:   "openshift-cluster",
-				Server: "https://api.my-cluster.example.com:6443",
+				Name:   "some-other-name",
+				Server: "https://my-cluster.example.com:6443",
 			},
 		},
 	}
 	apiResources := []string{
 		"pods",
 		"services",
 		"clusterversions.config.openshift.io",
+		"clusteroperators.config.openshift.io",
 	}
 
 	result := DetectClusterType(config, apiResources)
-	if result != types.ClusterTypeUnknown {
-		t.Errorf("DetectClusterType() = %v (%s), want %v", result, result.String(), types.ClusterTypeUnknown)
-	}
+	assert.Equal(t, types.ClusterTypeOpenShift4, result)
 }
 
 func TestDetectClusterType_OpenShift4_NoAPIResources(t *testing.T) {
@@ -295,8 +294,13 @@ func TestClusterTypeString(t *testing.T) {
 			want:        "GKE",
 		},
 		{
-			name:        "types.ClusterTypeInfraOpenShift4",
+			name:        "InfraOpenShift4",
 			clusterType: types.ClusterTypeInfraOpenShift4,
+			want:        "OpenShift4 (infra)",
+		},
+		{
+			name:        "OpenShift4",
+			clusterType: types.ClusterTypeOpenShift4,
 			want:        "OpenShift4",
 		},
 		{

internal/k8s/resource.go

@@ -64,7 +64,7 @@ func RetrieveResourceFromCluster(ctx context.Context, log *logger.Logger, namesp
 
 // IsResourceNotFound checks if an error is a resource not found error.
 func IsResourceNotFound(err error) bool {
-	return err == ErrResourceNotFound
+	return errors.Is(err, ErrResourceNotFound)
 }
 
 // ResourceNotOwnedByName checks if the given unstructured object does not have an owner reference with the specified owner name.
@@ -80,3 +80,33 @@ func ResourceNotOwnedByName(obj *unstructured.Unstructured, ownerName string) bo
 	}
 	return true
 }
+
+// RetrieveClusterResourceLabel retrieves the specified label from a Kubernetes resource from the cluster.
+//
+// Parameters: Same as for RetrieveResourceFromCluster, plus
+//   - label: The label to retrieve.
+//
+// Returns:
+//   - string: the label value -- if the label is not found, an empty string is returned without error.
+//   - error: nil if successful, error otherwise (including ErrResourceNotFound for not found resources)
+func RetrieveClusterResourceLabel(ctx context.Context, log *logger.Logger, namespace, resourceType, resourceName, label string) (string, error) {
+	u, err := RetrieveResourceFromCluster(ctx, log, namespace, resourceType, resourceName)
+	if err != nil {
+		return "", fmt.Errorf("retrieving resource %s/%s from namespace %s: %w", resourceType, resourceName, namespace, err)
+	}
+
+	val, found, err := unstructured.NestedFieldCopy(u.Object, "metadata", "labels", label)
+	if err != nil {
+		return "", fmt.Errorf("extracting label value: %w", err)
+	}
+	if !found {
+		return "", nil
+	}
+
+	valStr, ok := val.(string)
+	if !ok {
+		return "", fmt.Errorf("unexpected type for label %q (%T)", label, val)
+	}
+
+	return valStr, nil
+}

internal/types/cluster_type.go

@@ -10,6 +10,8 @@ const (
 	ClusterTypeInfraGKE
 	// ClusterTypeInfraOpenShift4 represents an OpenShift 4 cluster
 	ClusterTypeInfraOpenShift4
+	// Generic OpenShift4 cluster (e.g. for prow CI)
+	ClusterTypeOpenShift4
 	// ClusterTypeKind represents a Kind (Kubernetes in Docker) cluster
 	ClusterTypeKind
 	// ClusterTypeMinikube represents a Minikube cluster
@@ -20,12 +22,18 @@ const (
 	ClusterTypeCRC
 )
 
+func (ct ClusterType) IsOpenShift() bool {
+	return ct == ClusterTypeInfraOpenShift4 || ct == ClusterTypeOpenShift4
+}
+
 // String returns the string representation of a ClusterType
 func (ct ClusterType) String() string {
 	switch ct {
 	case ClusterTypeInfraGKE:
 		return "GKE"
 	case ClusterTypeInfraOpenShift4:
+		return "OpenShift4 (infra)"
+	case ClusterTypeOpenShift4:
 		return "OpenShift4"
 	case ClusterTypeKind:
 		return "Kind"
@@ -48,5 +56,6 @@ func AllClusterTypes() []ClusterType {
 		ClusterTypeK3s,
 		ClusterTypeCRC,
 		ClusterTypeInfraOpenShift4,
+		ClusterTypeOpenShift4,
 	}
 }