Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/validation"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -275,16 +274,16 @@ func configureWebhookServingCerts(cfg *rest.Config, cli client.Client, enableWeb
var webhookTLSConfigure = webhooktls.Configure

func resolveApplicationsNamespace() string {
provided := strings.TrimSpace(os.Getenv("APPLICATIONS_NAMESPACE"))
if provided != "" && len(validation.IsDNS1123Label(provided)) == 0 {
return provided
provided := os.Getenv("APPLICATIONS_NAMESPACE")
if valid := platform.ValidApplicationsNamespace(provided); valid != "" {
return valid
}

// Leave empty so the reconciler can fall back from Workbenches.spec.platform
// (opendatahub vs redhat-ods-applications). Product installs always inject the env.
// setupLog is intentional here: this runs during process startup before a
// reconcile context exists.
if provided == "" {
if strings.TrimSpace(provided) == "" {
setupLog.Info("APPLICATIONS_NAMESPACE not set; reconciler will use platform default")
} else {
setupLog.Info("APPLICATIONS_NAMESPACE invalid; reconciler will use platform default",
Expand Down
15 changes: 15 additions & 0 deletions internal/controller/platform_config_predicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ func TestPlatformConfigWatchNamespaces(t *testing.T) {
}
})

t.Run("invalid configured namespace falls back to both platform defaults", func(t *testing.T) {
t.Parallel()

r := &WorkbenchesReconciler{ApplicationsNamespace: "bad/name"}
got := r.platformConfigWatchNamespaces()
if len(got) != 2 {
t.Fatalf("platformConfigWatchNamespaces() len = %d, want 2", len(got))
}
if got[0] != platform.DefaultApplicationsNamespaceODH ||
got[1] != platform.DefaultApplicationsNamespaceRHOAI {
t.Fatalf("platformConfigWatchNamespaces() = %#v, want [%s %s]",
got, platform.DefaultApplicationsNamespaceODH, platform.DefaultApplicationsNamespaceRHOAI)
}
})

t.Run("unset falls back to both platform defaults", func(t *testing.T) {
t.Parallel()

Expand Down
21 changes: 13 additions & 8 deletions internal/controller/workbenches_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand Down Expand Up @@ -197,8 +196,8 @@ func (r *WorkbenchesReconciler) SetupWithManager(mgr ctrl.Manager) error {
// only that namespace; otherwise watch both platform defaults until CR platform
// selects one.
func (r *WorkbenchesReconciler) platformConfigWatchNamespaces() []string {
if r.ApplicationsNamespace != "" {
return []string{r.ApplicationsNamespace}
if ns := r.configuredApplicationsNamespace(); ns != "" {
return []string{ns}
}

return []string{
Expand All @@ -207,6 +206,13 @@ func (r *WorkbenchesReconciler) platformConfigWatchNamespaces() []string {
}
}

// configuredApplicationsNamespace returns ApplicationsNamespace when it is a valid
// DNS-1123 label. Invalid values are ignored so reconcile and ConfigMap watches
// fall back to platform defaults consistently.
func (r *WorkbenchesReconciler) configuredApplicationsNamespace() string {
return platform.ValidApplicationsNamespace(r.ApplicationsNamespace)
}

func shouldWatchImageStreams(mapper meta.RESTMapper) (bool, error) {
_, err := mapper.RESTMapping(gvk.ImageStream.GroupKind(), gvk.ImageStream.Version)
if err == nil {
Expand Down Expand Up @@ -670,13 +676,13 @@ func (r *WorkbenchesReconciler) setReadyCondition(

func (r *WorkbenchesReconciler) configureDependencies(ctx context.Context, wb *componentsv1alpha1.Workbenches) error {
appsNS := r.resolveOperandNamespace(wb.Spec.Platform)
if err := r.ensureGeneratedNamespace(ctx, wb, appsNS, "applications"); err != nil {
if err := r.ensureGeneratedNamespace(ctx, appsNS, "applications"); err != nil {
return fmt.Errorf("applications namespace: %w", err)
}

legacyNS := r.resolveLegacyWorkbenchNamespace(wb)
if legacyNS != appsNS {
if err := r.ensureGeneratedNamespace(ctx, wb, legacyNS, "legacy workbench"); err != nil {
if err := r.ensureGeneratedNamespace(ctx, legacyNS, "legacy workbench"); err != nil {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return fmt.Errorf("legacy workbench namespace: %w", err)
}
}
Expand All @@ -686,7 +692,6 @@ func (r *WorkbenchesReconciler) configureDependencies(ctx context.Context, wb *c

func (r *WorkbenchesReconciler) ensureGeneratedNamespace(
ctx context.Context,
_ *componentsv1alpha1.Workbenches,
nsName, purpose string,
) error {
l := log.FromContext(ctx)
Expand Down Expand Up @@ -757,8 +762,8 @@ func (r *WorkbenchesReconciler) setStatusNamespaces(wb *componentsv1alpha1.Workb
// fall back by platform: opendatahub (ODH/default) or redhat-ods-applications
// (SelfManagedRhoai). Spec.WorkbenchNamespace is not used for operand deploy.
func (r *WorkbenchesReconciler) resolveOperandNamespace(platformType string) string {
if r.ApplicationsNamespace != "" && len(validation.IsDNS1123Label(r.ApplicationsNamespace)) == 0 {
return r.ApplicationsNamespace
if ns := r.configuredApplicationsNamespace(); ns != "" {
return ns
}

return platform.DefaultApplicationsNamespace(platformType)
Expand Down
27 changes: 27 additions & 0 deletions internal/controller/workbenches_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ var _ = Describe("Workbenches Controller", func() {
DeferCleanup(func() {
cleanupWorkbenches(wb)
cleanupNamespace("legacy-invalid-apps-ns")
// fallbackNS is the shared suite applications namespace — do not delete it.
removeOwnedNamespaceLabel(fallbackNS)
})

_, err := reconcileWorkbenches(invalidReconciler, wb)
Expand Down Expand Up @@ -1079,6 +1081,31 @@ func cleanupNamespace(name string) {
ExpectWithOffset(1, k8sClient.Delete(ctx, ns)).To(Succeed())
}

func removeOwnedNamespaceLabel(name string) {
ns := &corev1.Namespace{}

err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, ns)
if client.IgnoreNotFound(err) != nil {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
return
}

if err != nil {
return
}

if ns.Labels == nil {
return
}

delete(ns.Labels, metadata.OwnedNamespaceLabel)
if len(ns.Labels) == 0 {
ns.Labels = nil
}

ExpectWithOffset(1, k8sClient.Update(ctx, ns)).To(Succeed())
}

func cleanupDeployments(namespace string) {
deployments := &appsv1.DeploymentList{}

Expand Down
17 changes: 17 additions & 0 deletions internal/platform/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ limitations under the License.
// Package platform provides platform type constants and helpers.
package platform

import (
"strings"

"k8s.io/apimachinery/pkg/util/validation"
)

// Platform type constants matching the orchestrator's platform identity values.
const (
OpenDataHub = "OpenDataHub"
Expand Down Expand Up @@ -61,6 +67,17 @@ func SectionTitle(platformType string) string {
return titles[OpenDataHub]
}

// ValidApplicationsNamespace returns name when it is a non-empty DNS-1123 label.
// Invalid or empty values return "" so callers can fall back to platform defaults.
func ValidApplicationsNamespace(name string) string {
name = strings.TrimSpace(name)
if name != "" && len(validation.IsDNS1123Label(name)) == 0 {
return name
}

return ""
}

// DefaultApplicationsNamespace returns the fallback applications namespace
// when APPLICATIONS_NAMESPACE is unset: opendatahub for ODH (and unknown),
// redhat-ods-applications for SelfManagedRhoai.
Expand Down
15 changes: 15 additions & 0 deletions internal/platform/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,18 @@ var _ = Describe("DefaultLegacyWorkbenchNamespace", func() {
Entry("empty defaults to ODH", "", platform.LegacyWorkbenchNamespaceODH),
)
})

var _ = Describe("ValidApplicationsNamespace", func() {
DescribeTable("accepts DNS-1123 labels only",
func(provided, want string) {
Expect(platform.ValidApplicationsNamespace(provided)).To(Equal(want))
},
Entry("empty", "", ""),
Entry("whitespace", " ", ""),
Entry("invalid slash", "bad/name", ""),
Entry("invalid underscore", "Invalid_Namespace", ""),
Entry("valid ODH default", "opendatahub", "opendatahub"),
Entry("valid RHOAI default", "redhat-ods-applications", "redhat-ods-applications"),
Entry("trimmed valid", " custom-apps ", "custom-apps"),
)
})