Skip to content

Flavours#2415

Draft
gazarenkov wants to merge 21 commits intoredhat-developer:mainfrom
gazarenkov:flavour2
Draft

Flavours#2415
gazarenkov wants to merge 21 commits intoredhat-developer:mainfrom
gazarenkov:flavour2

Conversation

@gazarenkov
Copy link
Member

Description

Introduce Flavors as [specified in ADR](Introduce Flavors as specified: https://docs.google.com/document/d/1CgxFZt7ITVb8LsbxZ1YRJP5CZZw7WMIZA3I_vUL8SmE/edit?tab=t.0

Which issue(s) does this PR fix or relate to

https://issues.redhat.com/browse/RHIDP-11505

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHIDP-11505 - Partially compliant

Compliant requirements:

  • Provide the necessary API/CRD types to configure flavours on the Backstage CR.

Non-compliant requirements:

  • Documentation updates (per PR acceptance criteria).

Requires further human verification:

  • Implement initial Flavours support according to the ADR (needs validation that merging/enablement semantics match ADR, including precedence/order and defaults).
  • Ensure behaviour is covered by tests (CI run + validate new tests truly assert intended semantics).
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Wrong Status

setDeploymentStatus always sets the condition status to False even when the resolved state is Deployed. This likely prevents the CR from ever reporting status.conditions[type=Deployed].status=True, which may break consumers that rely on a True deployed condition.

func (r *BackstageReconciler) setDeploymentStatus(ctx context.Context, backstage *api.Backstage, backstageModel model.BackstageModel) {
	var obj client.Object
	var resolveState func(client.Object) (api.BackstageConditionReason, string)

	switch backstageModel.GetDeploymentGVK() {
	case appsv1.SchemeGroupVersion.WithKind("StatefulSet"):
		obj = &appsv1.StatefulSet{}
		resolveState = func(o client.Object) (api.BackstageConditionReason, string) {
			return statefulSetState(o.(*appsv1.StatefulSet))
		}
	default:
		obj = &appsv1.Deployment{}
		resolveState = func(o client.Object) (api.BackstageConditionReason, string) {
			return deploymentState(o.(*appsv1.Deployment))
		}
	}

	if err := r.Get(ctx, types.NamespacedName{Name: model.DeploymentName(backstage.Name), Namespace: backstage.GetNamespace()}, obj); err != nil {
		setStatusCondition(backstage, api.BackstageConditionTypeDeployed, metav1.ConditionFalse, api.BackstageConditionReasonFailed, err.Error())
		return
	}

	state, msg := resolveState(obj)
	setStatusCondition(backstage, api.BackstageConditionTypeDeployed, metav1.ConditionFalse, state, msg)
}
📚 Focus areas based on broader codebase context

Test Bug

The test claims "Assert exact match (order matters)" but uses assert.ElementsMatch, which ignores ordering and can let incorrect flavour precedence pass. Change the assertion to an order-sensitive check (e.g., assert.Equal(tt.wantFlavours, gotNames)) so merge/override order is actually validated. (Ref 6, Ref 4)

	// Extract flavour names
	gotNames := make([]string, len(flavours))
	for i, f := range flavours {
		gotNames[i] = f.name
	}

	// Assert exact match (order matters)
	if !assert.ElementsMatch(t, tt.wantFlavours, gotNames) {
		t.Logf("Expected flavours: %v", tt.wantFlavours)
		t.Logf("Got flavours: %v", gotNames)
	}
}

Reference reasoning: Existing tests in the codebase validate order explicitly by asserting exact positions in slices (e.g., checking container args indices with Equal), rather than using order-insensitive matchers. Aligning this test with that pattern will correctly enforce the intended ordering semantics for flavour merging.

📄 References
  1. redhat-developer/rhdh-operator/tests/e2e/e2e_suite_test.go [54-61]
  2. redhat-developer/rhdh-operator/tests/e2e/e2e_suite_test.go [63-91]
  3. redhat-developer/rhdh-operator/tests/e2e/e2e_suite_test.go [93-112]
  4. redhat-developer/rhdh-operator/tests/e2e/e2e_test.go [58-94]
  5. redhat-developer/rhdh-operator/integration_tests/rhdh-config_test.go [142-181]
  6. redhat-developer/rhdh-operator/integration_tests/rhdh-config_test.go [96-120]
  7. redhat-developer/rhdh-operator/integration_tests/rhdh-config_test.go [261-278]
  8. redhat-developer/rhdh-operator/integration_tests/cr-config_test.go [120-148]

Co-authored-by: gazarenkov <gazarenkov@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

⚠️ Files changed in bundle and installer generation!

Those changes to the operator bundle/installer manifests should have been pushed automatically to your PR branch.

NOTE: If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.

@rhdh-qodo-merge rhdh-qodo-merge bot added enhancement New feature or request Tests labels Mar 2, 2026
@rhdh-qodo-merge
Copy link

PR Type

Enhancement, Tests


Description

  • Introduces Flavours feature as a new capability for pre-configured domain-specific customizations in Backstage deployments

  • Adds new v1alpha6 API version with Flavour type supporting Name and Enabled fields

  • Implements flavour discovery, metadata loading, and merging logic with three merge policies: NoFlavour, ArrayMerge, and MultiObject

  • Creates API abstraction layer via api/current-types.go to decouple application code from specific API versions

  • Adds comprehensive test coverage for flavour functionality including GetEnabledFlavours() and dynamic plugin merging

  • Implements deployment status condition tracking with helper methods for checking local DB, routes, auth secrets, and monitoring

  • Updates all model files, controllers, and integration tests to use the new API abstraction layer

  • Auto-generates deepcopy implementations for all v1alpha6 types


File Walkthrough

Relevant files
Enhancement
8 files
backstage_types.go
Add Flavours support and deployment status conditions       

api/v1alpha6/backstage_types.go

  • Introduces new Flavour type with Name and Enabled fields for
    pre-configured templates
  • Adds Flavours field to BackstageSpec to enable/disable domain-specific
    customizations
  • Defines condition types and reasons for deployment status tracking
  • Implements helper methods IsLocalDbEnabled(), IsRouteEnabled(),
    IsAuthSecretSpecified(), and IsMonitoringEnabled()
+393/-0 
dynamic-plugins.go
Extract plugin merging logic and add flavour merge policy

pkg/model/dynamic-plugins.go

  • Adds FlavourMergePolicyNoFlavour parameter to registerConfig() call
  • Extracts MergePluginsData() as standalone function for reusable plugin
    merging logic
  • Refactors mergeWith() to use the new MergePluginsData() function
  • Updates imports from bsv1 to generic api package
+55/-39 
default-config.go
Add flavour-aware default config reading and merging         

pkg/model/default-config.go

  • Introduces ReadDefaultConfig() as main entry point for reading configs
    with flavour support
  • Implements mergeDynamicPlugins() for merging dynamic-plugins.yaml
    files by package name
  • Implements mergeMultiObjectConfigs() for handling multi-object configs
    with flavour prefixing
  • Provides helper functions for collecting config paths and sources from
    flavours
+192/-0 
flavour.go
Add flavour discovery and metadata loading logic                 

pkg/model/flavour.go

  • Introduces FlavourMetadata type for reading flavour metadata.yaml
    files
  • Implements GetEnabledFlavours() to determine enabled flavours based on
    spec and defaults
  • Implements mergeWithDefaults() to merge spec flavours with default
    flavours
  • Provides helper functions for loading flavour metadata and discovering
    default flavours
+151/-0 
runtime.go
Integrate flavour support into runtime object initialization

pkg/model/runtime.go

  • Updates registerConfig() to accept FlavourMergePolicy parameter
  • Calls GetEnabledFlavours() once per reconciliation to determine
    enabled flavours
  • Replaces direct utils.ReadYamlFiles() calls with ReadDefaultConfig()
    for flavour support
  • Updates imports from bsv1 to generic api package
+20/-9   
current-types.go
API version abstraction layer for decoupling                         

api/current-types.go

  • New file introducing type aliases to decouple application code from
    specific API versions
  • Aliases point to v1alpha6 types for Backstage, BackstageSpec, and
    related types
  • Provides centralized location for API version upgrades
  • Includes AddToScheme function delegation to underlying versioned API
+60/-0   
groupversion_info.go
v1alpha6 API group version initialization                               

api/v1alpha6/groupversion_info.go

  • New file defining v1alpha6 API group version schema
  • Sets up GroupVersion, SchemeBuilder, and AddToScheme for the new API
    version
  • Establishes group name as rhdh.redhat.com with version v1alpha6
+20/-0   
interfaces.go
Flavour merge policy framework for configuration                 

pkg/model/interfaces.go

  • Added FlavourMergePolicy type with three policy constants for config
    merging strategies
  • FlavourMergePolicyNoFlavour for core infrastructure configs
  • FlavourMergePolicyArrayMerge for array-based merging (e.g., plugins)
  • FlavourMergePolicyMultiObject for separate flavour-specific objects
  • Updated ObjectConfig struct to include FlavourMergePolicy field
  • Changed import from bsv1 to api package
+26/-5   
Code generation
1 files
zz_generated.deepcopy.go
Auto-generated deepcopy implementations for v1alpha6         

api/v1alpha6/zz_generated.deepcopy.go

  • Auto-generated deepcopy functions for all v1alpha6 types
  • Implements DeepCopyInto() and DeepCopy() methods for proper object
    cloning
  • Handles nested structures including Flavour, Monitoring, and complex
    field types
+467/-0 
Tests
1 files
default-config_test.go
Add tests for flavours and dynamic plugins merging             

pkg/model/default-config_test.go

  • Tests for mergeDynamicPlugins() function covering empty paths, single
    path, and multi-path merging
  • Tests for GetEnabledFlavours() function with various flavour
    combinations and defaults
  • Validates flavour ordering, explicit enable/disable, and error
    handling for nonexistent flavours
+267/-0 
Refactoring
41 files
route_test.go
Update imports to use generic api package                               

pkg/model/route_test.go

  • Updates imports from bsv1 to generic api package
  • Maintains all existing test logic for route configuration and URL
    building
+37/-37 
secretfiles_test.go
Update imports to use generic api package                               

pkg/model/secretfiles_test.go

  • Updates imports from bsv1 to generic api package throughout test file
  • Maintains all existing test logic for secret file mounting and
    validation
+22/-22 
cr-config_test.go
Update imports to use generic api package                               

integration_tests/cr-config_test.go

  • Updates imports from bsv1 to generic api package
  • Maintains all existing integration test logic for CR configuration
+28/-28 
backstage_status.go
Update imports to use generic api package                               

internal/controller/backstage_status.go

  • Updates imports from bsv1 to generic api package
  • Maintains all status condition handling logic for deployment and
    statefulset states
+17/-17 
backstage_controller.go
Update imports to use generic api package                               

internal/controller/backstage_controller.go

  • Updates imports from bsv1 to generic api package
  • Maintains all reconciliation logic and status condition updates
+8/-8     
configmapfiles_test.go
Update imports to use generic api package                               

pkg/model/configmapfiles_test.go

  • Updates imports from bsv1 to generic api package throughout test file
  • Maintains all existing test logic for configmap file mounting and
    validation
+18/-18 
deployment_test.go
Update imports to use generic api package                               

pkg/model/deployment_test.go

  • Updates imports from bsv1 to generic api package throughout test file
  • Maintains all existing test logic for deployment configuration and
    patching
+15/-15 
configmapenvs_test.go
Update imports to use generic api package                               

pkg/model/configmapenvs_test.go

  • Updates imports from bsv1 to generic api package throughout test file
  • Maintains all existing test logic for configmap environment variable
    injection
+23/-23 
runtime_test.go
Update imports to use generic api package                               

pkg/model/runtime_test.go

  • Updates imports from bsv1 to generic api package throughout test file
  • Maintains all existing test logic for runtime object initialization
+15/-15 
default-config_test.go
Update imports to use generic api package                               

integration_tests/default-config_test.go

  • Updates imports from bsv1 to generic api package
  • Maintains all existing integration test logic for default
    configuration
+8/-8     
config-refresh_test.go
Update imports to use generic api package                               

integration_tests/config-refresh_test.go

  • Updates imports from bsv1 to generic api package
  • Maintains all existing integration test logic for configuration
    refresh
+15/-15 
db_test.go
Update imports to use new API abstraction layer                   

integration_tests/db_test.go

  • Updated import from bsv1
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Changed all type references from bsv1.BackstageSpec, bsv1.Backstage,
    bsv1.Database to api.*
  • Updated 8 test cases to use new API package imports
+8/-8     
route.go
Update route model to use API abstraction                               

pkg/model/route.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.Route and api.Backstage types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+8/-13   
pvcs_test.go
Update PVCs integration tests to new API                                 

integration_tests/pvcs_test.go

  • Updated import from bsv1 to api package
  • Changed all type references to use api.* instead of bsv1.*
  • Updated 3 test cases with new API imports
+10/-10 
route_test.go
Update route integration tests to new API                               

integration_tests/route_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.Route, bsv1.BackstageSpec,
    bsv1.Backstage, bsv1.Application to api.*
  • Updated 4 test cases with new API imports
+8/-8     
deployment.go
Update deployment model to use API abstraction                     

pkg/model/deployment.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.Backstage and api.ExtraEnvs
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
+7/-7     
pvcs_test.go
Update PVCs unit tests to new API                                               

pkg/model/pvcs_test.go

  • Updated import from bsv1 to api package
  • Changed all type references from bsv1.* to api.* in 4 test functions
+16/-16 
suite_test.go
Update test suite to use API abstraction                                 

integration_tests/suite_test.go

  • Updated import order, moving api before versioned imports
  • Changed from bsv1
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Updated function signatures and type references to use
    api.BackstageSpec and api.Backstage
  • Changed bsv1.AddToScheme to api.AddToScheme
+6/-6     
secretenvs_test.go
Update secret environment tests to new API                             

pkg/model/secretenvs_test.go

  • Updated import from bsv1 to api package
  • Changed all type references from bsv1.* to api.* in test variables and
    functions
+14/-14 
rhdh-config_test.go
Update RHDH config tests to new API                                           

integration_tests/rhdh-config_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.BackstageSpec, bsv1.Backstage,
    bsv1.Application, bsv1.AppConfig, bsv1.ExtraFiles, bsv1.FileObjectRef
    to api.*
+7/-7     
pvcs.go
Update PVCs model to use API abstraction                                 

pkg/model/pvcs.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-10   
configmapenvs.go
Update ConfigMap environment model to new API                       

pkg/model/configmapenvs.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-11   
configmapfiles.go
Update ConfigMap files model to new API                                   

pkg/model/configmapfiles.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-11   
appconfig_test.go
Update app config tests to new API                                             

pkg/model/appconfig_test.go

  • Updated import from bsv1 to api package
  • Changed all type references from bsv1.* to api.* in test variables and
    functions
+10/-10 
monitor_test.go
Update monitor tests to use API abstraction                           

internal/controller/monitor_test.go

  • Updated import from bs
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Changed type references from bs.* to api.*
  • Updated bs.AddToScheme to api.AddToScheme
+7/-7     
db-statefulset.go
Update database StatefulSet model to new API                         

pkg/model/db-statefulset.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.Backstage type
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+5/-10   
secretenvs.go
Update secret environment model to new API                             

pkg/model/secretenvs.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-11   
appconfig.go
Update app config model to new API                                             

pkg/model/appconfig.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-11   
secretfiles.go
Update secret files model to new API                                         

pkg/model/secretfiles.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.BackstageSpec and api.Backstage
    types
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
  • Removed commented-out EmptyObject method
+6/-11   
db-secret.go
Update database secret model to new API                                   

pkg/model/db-secret.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.Backstage type
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
+5/-5     
db-service.go
Update database service model to new API                                 

pkg/model/db-service.go

  • Updated import from bsv1 to api package
  • Changed function signatures to use api.Backstage type
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
+5/-5     
model_tests.go
Update model test helpers to new API                                         

pkg/model/model_tests.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.Backstage to api.Backstage
  • Updated bsv1.AddToScheme to api.AddToScheme
+5/-5     
plugin-deps_test.go
Update plugin dependencies tests to new API                           

integration_tests/plugin-deps_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.BackstageSpec and bsv1.RuntimeConfig
    to api.*
+3/-3     
service.go
Update service model to v1alpha6 API                                         

pkg/model/service.go

  • Updated import from bsv1 to bsv1
    "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
  • Added FlavourMergePolicyNoFlavour parameter to registerConfig call
+2/-7     
monitor.go
Update monitor controller to use API abstraction                 

internal/controller/monitor.go

  • Updated import from bs
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Changed function signature to use api.Backstage type
+2/-2     
plugin_deps_test.go
Update plugin dependencies unit tests to new API                 

pkg/model/plugin_deps_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.Backstage to api.Backstage
  • Updated bsv1.AddToScheme to api.AddToScheme
+3/-3     
plugin-deps.go
Update plugin dependencies controller to new API                 

internal/controller/plugin-deps.go

  • Updated import from bs
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Changed function signature to use api.Backstage type
+2/-2     
db-statefulset_test.go
Update database StatefulSet tests to new API                         

pkg/model/db-statefulset_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.* to api.* in test variables
+5/-5     
watchers.go
Update watchers controller to use API abstraction               

internal/controller/watchers.go

  • Updated import from bs
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to
    "github.com/redhat-developer/rhdh-operator/api"
  • Changed type reference from bs.Backstage to api.Backstage
+2/-2     
main.go
Update main command to use v1alpha6 API                                   

cmd/main.go

  • Updated import from bsv1
    "github.com/redhat-developer/rhdh-operator/api/v1alpha5" to bsv1
    "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+1/-1     
db-secret_test.go
Update database secret tests to new API                                   

pkg/model/db-secret_test.go

  • Updated import from bsv1 to api package
  • Changed type references from bsv1.* to api.* in test variables
+4/-4     
Additional files
34 files
airgap.adoc +1/-1     
install-rhdh-catalog-source.sh +1/-1     
prepare-restricted-environment.sh +1/-1     
PROJECT +2/-2     
backstage_types.go +0/-1     
backstage-operator.clusterserviceversion.yaml +1/-1     
backstage-operator.clusterserviceversion.yaml +1/-1     
rhdh.redhat.com_backstages.yaml +513/-0 
_v1alpha6_backstage.yaml +8/-0     
kustomization.yaml +1/-0     
configuration.md +1/-1     
dynamic-plugins.md +1/-1     
external-db.md +1/-1     
monitoring.md +5/-5     
bs-existing-secret.yaml +1/-1     
bs-route-disabled.yaml +1/-1     
bs-route.yaml +1/-1     
bs1.yaml +1/-1     
catalog-index.yaml +1/-1     
envvars.yaml +1/-1     
filemounts.yaml +1/-1     
orchestrator-cicd.yaml +1/-1     
orchestrator.yaml +1/-1     
pvc-dp-cache.yaml +1/-1     
raw-runtime-config.yaml +1/-1     
rhdh-cr-with-app-configs.yaml +1/-1     
rhdh-cr.yaml +1/-1     
cr-compatibility_test.go +7/-7     
rhdh-replace-dynaplugin-root.yaml +1/-1     
spec-deployment.yaml +1/-1     
preprocessor_test.go +1/-1     
spec_preprocessor.go +1/-1     
dynamic-plugins_test.go +8/-8     
utils.go +0/-61   

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Mar 2, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Flavour discovery relies on filesystem path

The flavour discovery mechanism should not rely on the LOCALBIN environment
variable and a filesystem path. Instead, it should use a more robust method like
embedding flavour configurations into the operator binary using Go's embed
feature.

Examples:

pkg/model/flavour.go [36-37]

Solution Walkthrough:

Before:

// pkg/model/flavour.go

func GetEnabledFlavours(spec api.BackstageSpec) ([]enabledFlavour, error) {
    // Relies on an environment variable to find the flavours directory.
    localBin := os.Getenv("LOCALBIN")
    flavoursDir := filepath.Join(localBin, "default-config", "flavours")

    // Reads from the filesystem path, which might fail silently if LOCALBIN is wrong.
    defaults, err := getDefaultFlavours(flavoursDir)
    if err != nil {
        return nil, err
    }
    // ... more logic using flavoursDir
    return mergeWithDefaults(spec.Flavours, defaults, flavoursDir)
}

After:

// pkg/model/flavour.go
import "embed"

//go:embed default-config/flavours
var flavoursFS embed.FS

func GetEnabledFlavours(spec api.BackstageSpec) ([]enabledFlavour, error) {
    // Path is now relative to the embedded filesystem, not an external variable.
    flavoursDir := "default-config/flavours"

    // Reads from the embedded filesystem, making it self-contained and reliable.
    defaults, err := getDefaultFlavours(flavoursDir)
    if err != nil {
        return nil, err
    }
    // ...
    return mergeWithDefaults(spec.Flavours, defaults, flavoursDir)
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant design flaw in the new flavour discovery mechanism, which relies on a fragile environment variable (LOCALBIN) and filesystem path, proposing robust, standard alternatives.

Medium
General
Enable flavour‐aware plugin merging

Change the flavour merge policy for dynamic-plugins.yaml from
FlavourMergePolicyNoFlavour to FlavourMergePolicyArrayMerge to enable merging of
plugins across different flavours.

pkg/model/runtime.go [64]

 func init() {
-    registerConfig("dynamic-plugins.yaml", DynamicPluginsFactory{}, false, FlavourMergePolicyNoFlavour)
+    registerConfig("dynamic-plugins.yaml", DynamicPluginsFactory{}, false, FlavourMergePolicyArrayMerge)
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This suggestion corrects a significant configuration error, enabling the newly introduced "flavours" feature to correctly apply to dynamic plugins, which is a key part of the PR's intent.

Medium
Use version-agnostic API import alias
Suggestion Impact:The commit updated the import from api/v1alpha6 to api and adjusted related type references from bsv1.Backstage to api.Backstage accordingly.

code diff:

@@ -5,7 +5,7 @@
 
 	"k8s.io/apimachinery/pkg/runtime"
 
-	bsv1 "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	"github.com/redhat-developer/rhdh-operator/api"
 	"github.com/redhat-developer/rhdh-operator/pkg/utils"
 
 	corev1 "k8s.io/api/core/v1"
@@ -43,7 +43,7 @@
 }
 
 // implementation of RuntimeObject interface
-func (b *BackstageService) addToModel(model *BackstageModel, _ bsv1.Backstage) (bool, error) {
+func (b *BackstageService) addToModel(model *BackstageModel, _ api.Backstage) (bool, error) {
 	b.model = model
 	if b.service == nil {
 		return false, fmt.Errorf("backstage Service is not initialized, make sure there is service.yaml in default or raw configuration")
@@ -56,11 +56,11 @@
 }
 
 // implementation of RuntimeObject interface
-func (b *BackstageService) updateAndValidate(_ bsv1.Backstage) error {
+func (b *BackstageService) updateAndValidate(_ api.Backstage) error {
 	return nil
 }
 
-func (b *BackstageService) setMetaInfo(backstage bsv1.Backstage, scheme *runtime.Scheme) {
+func (b *BackstageService) setMetaInfo(backstage api.Backstage, scheme *runtime.Scheme) {
 	b.service.SetName(ServiceName(backstage.Name))
 	utils.GenerateLabel(&b.service.Spec.Selector, BackstageAppLabel, utils.BackstageAppLabelValue(backstage.Name))
 	setMetaInfo(b.service, backstage, scheme)

Replace the version-specific import bsv1
"github.com/redhat-developer/rhdh-operator/api/v1alpha6" with the
version-agnostic alias api "github.com/redhat-developer/rhdh-operator/api".

pkg/model/service.go [6-12]

 import (
 	"k8s.io/apimachinery/pkg/runtime"
 
-	bsv1 "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	"github.com/redhat-developer/rhdh-operator/api"
 	"github.com/redhat-developer/rhdh-operator/pkg/utils"
 
 	corev1 "k8s.io/api/core/v1"
 )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out an inconsistent import that goes against the PR's goal of using a version-agnostic API alias, improving code consistency and maintainability.

Low
Switch to alias API import
Suggestion Impact:Updated the import from github.com/redhat-developer/rhdh-operator/api/v1alpha6 to github.com/redhat-developer/rhdh-operator/api and adjusted the Backstage type reference in GetPluginDeps to use api.Backstage.

code diff:

@@ -6,7 +6,7 @@
 	"path/filepath"
 	"strings"
 
-	bs "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	"github.com/redhat-developer/rhdh-operator/api"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -15,7 +15,7 @@
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-func GetPluginDeps(backstage bs.Backstage, plugins DynamicPlugins, scheme *runtime.Scheme) ([]*unstructured.Unstructured, error) {
+func GetPluginDeps(backstage api.Backstage, plugins DynamicPlugins, scheme *runtime.Scheme) ([]*unstructured.Unstructured, error) {
 

Replace the direct import of the versioned API api/v1alpha6 with the central
alias package github.com/redhat-developer/rhdh-operator/api.

pkg/model/plugin_deps.go [6-12]

 import (
 	"path/filepath"
 	"strings"
 
-	bs "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	api "github.com/redhat-developer/rhdh-operator/api"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies an inconsistent import that deviates from the PR's refactoring pattern, and fixing it improves code consistency and maintainability.

Low
Import central API alias
Suggestion Impact:The commit switched the import from the versioned v1alpha6 API to the central api package and updated type references from bsv1.* to api.* (e.g., Backstage, Application, FileObjectRef).

code diff:

@@ -24,7 +24,7 @@
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	bsv1 "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	"github.com/redhat-developer/rhdh-operator/api"
 
 	"github.com/redhat-developer/rhdh-operator/pkg/model"
 
@@ -34,7 +34,7 @@
 
 // Add additional details to the Backstage Spec helping in making Backstage RuntimeObjects Model
 // Validates Backstage Spec and fails fast if something not correct
-func (r *BackstageReconciler) preprocessSpec(ctx context.Context, backstage bsv1.Backstage) (model.ExternalConfig, error) {
+func (r *BackstageReconciler) preprocessSpec(ctx context.Context, backstage api.Backstage) (model.ExternalConfig, error) {
 	//lg := log.FromContext(ctx)
 
 	bsSpec := backstage.Spec
@@ -75,7 +75,7 @@
 	}
 
 	if bsSpec.Application == nil {
-		bsSpec.Application = &bsv1.Application{}
+		bsSpec.Application = &api.Application{}
 	}
 
 	// Process AppConfigs
@@ -222,7 +222,7 @@
 	return nil
 }
 
-func addToWatch(fileObjectRef bsv1.FileObjectRef) bool {
+func addToWatch(fileObjectRef api.FileObjectRef) bool {
 	// it will contain subPath either as specified key or as a list of all keys if only mountPath specified

Replace the versioned API import bsv1
"github.com/redhat-developer/rhdh-operator/api/v1alpha6" with the central alias
api "github.com/redhat-developer/rhdh-operator/api".

internal/controller/spec_preprocessor.go [24-30]

 import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	bsv1 "github.com/redhat-developer/rhdh-operator/api/v1alpha6"
+	api "github.com/redhat-developer/rhdh-operator/api"
 
 	"github.com/redhat-developer/rhdh-operator/pkg/model"
 )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies an inconsistent import that deviates from the PR's refactoring pattern, and fixing it improves code consistency and maintainability.

Low
Possible issue
Align implementation with API documentation

Modify GetEnabledFlavours to handle an empty spec.Flavours slice by returning no
flavours, aligning the function's behavior with its API documentation.

pkg/model/flavour.go [35-49]

 func GetEnabledFlavours(spec api.BackstageSpec) ([]enabledFlavour, error) {
 	localBin := os.Getenv("LOCALBIN")
 	flavoursDir := filepath.Join(localBin, "default-config", "flavours")
 
 	// Get all default flavours
 	defaults, err := getDefaultFlavours(flavoursDir)
 	if err != nil {
 		return nil, err
-	} else if spec.Flavours == nil {
+	}
+
+	if spec.Flavours == nil {
 		return defaults, nil
+	}
+
+	// Empty array disables all flavours
+	if len(spec.Flavours) == 0 {
+		return []enabledFlavour{}, nil
 	}
 
 	// Merge spec.Flavours with defaults
 	return mergeWithDefaults(spec.Flavours, defaults, flavoursDir)
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a functional bug where the implementation contradicts the API documentation, preventing users from disabling all flavours as intended.

Medium
Prevent panic and incorrect metadata

Refactor mergeDynamicPlugins to prevent a panic on empty YAML files and to
correctly preserve the metadata from the base ConfigMap during a merge.

pkg/model/default-config.go [102-141]

 func mergeDynamicPlugins(paths []string, scheme runtime.Scheme, platformExt string) ([]client.Object, error) {
 
 	if len(paths) == 0 {
 		return []client.Object{}, nil
 	}
 
 	var resultConfigMap *corev1.ConfigMap
 	var mergedData string
 
-	for i := 0; i < len(paths); i++ {
-		path := paths[i]
-
+	for i, path := range paths {
 		objs, err := utils.ReadYamlFiles(path, scheme, platformExt)
 		if err != nil {
 			return nil, fmt.Errorf("failed to read dynamic-plugins.yaml from %s: %w", path, err)
+		}
+
+		if len(objs) == 0 {
+			continue // Skip empty files
 		}
 
 		// single object expected
 		configMap, ok := objs[0].(*corev1.ConfigMap)
 		if !ok {
 			return nil, fmt.Errorf("no ConfigMap found in %s", path)
 		}
 
+		if i == 0 {
+			resultConfigMap = configMap
+		}
+
 		data, ok := configMap.Data[DynamicPluginsFile]
 		if !ok {
-			return nil, fmt.Errorf("no %s key found in ConfigMap from %s", DynamicPluginsFile, path)
+			// It's valid for a configmap to not have the key, just skip it
+			continue
 		}
 
 		mergedData, err = MergePluginsData(mergedData, data)
 		if err != nil {
 			return nil, fmt.Errorf("failed to merge dynamic-plugins from %s: %w", path, err)
 		}
-		resultConfigMap = configMap
+	}
+
+	if resultConfigMap == nil {
+		return []client.Object{}, nil
 	}
 
 	// Update the ConfigMap with merged data
 	resultConfigMap.Data[DynamicPluginsFile] = mergedData
 
 	return []client.Object{resultConfigMap}, nil
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential panic if a YAML file is empty and a logic error where the final ConfigMap metadata would be incorrect, improving both robustness and correctness.

Medium
Use correct flavour merge policy
Suggestion Impact:The commit changed the app-config.yaml registration away from FlavourMergePolicyNoFlavour and refactored AppConfig to use a multi-object approach (MultiObject with a multi-object merge function), aligning with the intended multi-object flavour merge behavior, though implemented via a different API/signature than the suggested constant swap.

code diff:

 func init() {
-	registerConfig("app-config.yaml", AppConfigFactory{}, false, FlavourMergePolicyNoFlavour)
+	registerConfig("app-config.yaml", AppConfigFactory{}, true, mergeMultiObjectConfigs)
 }

Change the flavour merge policy for app-config.yaml from
FlavourMergePolicyNoFlavour to FlavourMergePolicyMultiObject to align with the
design specified in pkg/model/interfaces.go.

pkg/model/appconfig.go [29-31]

 func init() {
-	registerConfig("app-config.yaml", AppConfigFactory{}, false, FlavourMergePolicyNoFlavour)
+	registerConfig("app-config.yaml", AppConfigFactory{}, false, FlavourMergePolicyMultiObject)
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an inconsistency between the implementation and the design comments for the new 'Flavours' feature, which could be a potential bug.

Medium
  • Update

Co-authored-by: gazarenkov <gazarenkov@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

⚠️ Files changed in bundle and installer generation!

Those changes to the operator bundle/installer manifests should have been pushed automatically to your PR branch.

NOTE: If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 5, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant