feat(operator): remote-deployment

[akafazov]

Adds support for remote infra and runtime cluster operation.

infra - where the operator runs along with some deployment technology (FluxCD, ...)
runtime - cluster hosting the PlatformMesh resource and profile and where the actual workloads run

Also replaces the platform-mesh-operator-components and platform-mesh-operator-infra-components charts with a profile (values.yaml) and built-in gotemplates.

Changes:

• update operator to use SSA instead of Create or Update kubernetes operations
• update PlatformMesh CRD to include reference to profile ConfigMap, holding configuration replacing the platform-mesh-operator-components and platform-mesh-operator-infra-components Values.yaml; update CRD printer fields;
• operator configuration - new parameters remote-infra-enabled and remote-runtime-enabled
• gotemplates directory with template mirroring the helm templates from the charts above
• profile template - exposes configuration to the user. A default profile should be created in the operator chart.
• updated template replacement interface in the PlatformMesh resource from {{ .Values.baseDomain }} to {{ .baseDomain }}
• remove unused kcpUrl parameter
• creates resources in the namespace where the parent PlatformMesh resource is located
• updated README.md to reflect correct Remote Runtime Configuration parameter names matching the actual config.go structure
• fix wait subroutine and config

refers to platform-mesh/helm-charts#594

[akafazov]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[akafazov]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

Adds profile-driven deployment via a new PlatformMeshSpec.profileConfigMap and ConfigMap-based profile.yaml loading. Introduces many gotemplates for infra/runtime/components and replaces several bundled HelmRelease/Resource manifests. Implements dual-cluster clients (clientInfra, clientRuntime), RemoteInfra/RemoteRuntime operator config, and helpers to obtain REST configs. Subroutines and reconcilers now accept injected clients; resource flows use SSA/Server-Side Apply/Patch for updates. Adds template rendering/apply helpers, profile merging, numerous tests and e2e changes, a TEST_TIMEOUT Taskfile variable, .gitignore additions, and a Dockerfile copy of gotemplates.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)

internal/controller/realm_controller.go (1)

40-62: Use clientInfra instead of mgr.GetClient() for PatchOIDCSubroutine.

The PatchOIDCSubroutine at line 44 uses mgr.GetClient(), while other subroutines in the codebase (e.g., ResourceSubroutine and WaitSubroutine) use clientInfra for remote cluster operations. For consistency, PatchOIDCSubroutine should be initialized with clientInfra rather than the local cluster client.

pkg/subroutines/providersecret.go (2)

82-82: Inconsistent config source.

operatorCfg is loaded from context here, but r.cfgOperator is already available as a struct field and used elsewhere (e.g., lines 133, 230-232). This creates inconsistency. Consider using r.cfgOperator throughout for uniformity.

Proposed fix

-	operatorCfg := pmconfig.LoadConfigFromContext(ctx).(config.OperatorConfig)
+	operatorCfg := r.cfgOperator

Note: You may need to adjust subsequent usages to dereference the pointer if needed.

---

303-306: Same config inconsistency as in Process method.

operatorCfg is loaded from context instead of using r.cfgOperator. Apply the same fix here for consistency.

README.md (1)

14-14: Typo in example YAML.

spec:iam-service appears to be a typo. Should likely be spec: followed by proper indentation.

Proposed fix

-spec:iam-service
+spec:

pkg/subroutines/subroutine_helpers.go (1)

279-281: Ignored error from json.Marshal.

The error from json.Marshal on line 280 is discarded. While failure is unlikely for this map structure, suppressing errors can mask unexpected issues.

Proposed fix

 	result := apiextensionsv1.JSON{}
-	result.Raw, _ = json.Marshal(values)
+	raw, err := json.Marshal(values)
+	if err != nil {
+		return apiextensionsv1.JSON{}, errors.Wrap(err, "Failed to marshal template vars")
+	}
+	result.Raw = raw

 	return result, nil

🧹 Nitpick comments (21)

gotemplates/infra/runtime/gateway-api/resource.yaml (1)

1-29: Consider validating template data before rendering.

The template dereferences nested fields (e.g., .ocm.component.name, .gatewayApi.ocmResourceName) without validation. If the profile ConfigMap is missing required fields, rendering will panic. Consider adding validation in the operator code that loads and renders these templates to ensure all required fields are present.

gotemplates/components/runtime/ocm-image-resources.yaml (3)

17-22: Inconsistent whitespace control in template directives.

Line 17 uses {{- toYaml . | nindent 4 }} with whitespace trimming, but line 21 uses {{ toYaml . | nindent 4 }} without the leading trim marker. This inconsistency may introduce unintended blank lines.

🔎 Proposed fix for consistent whitespace handling

   {{- with .annotations }}
   annotations:
-{{ toYaml . | nindent 4 }}
+    {{- toYaml . | nindent 4 }}
   {{- end }}

---

29-29: Quote the default string value.

The default value for .resource should be quoted to maintain consistency with YAML string conventions.

🔎 Proposed fix

-        name: {{ .resource | default "image"}}
+        name: {{ .resource | default "image" }}

---

47-47: Trailing document separator may create empty YAML document.

The --- separator at line 47 will create an empty YAML document if this is the last imageResource. Consider moving the separator to precede each resource after the first.

api/v1alpha1/platformmesh_types.go (1)

36-44: ConfigMapReference type is well-structured.

The type correctly marks Namespace as optional with the +optional marker and includes clear documentation about the defaulting behavior. The design follows Kubernetes API conventions.

Consider adding a +kubebuilder:validation:MinLength=1 marker to the Name field to prevent empty strings, ensuring the reference is always valid when specified.

🔎 Proposed validation enhancement

 type ConfigMapReference struct {
 	// Name is the name of the ConfigMap.
+	// +kubebuilder:validation:MinLength=1
 	Name string `json:"name"`
 	// Namespace is the namespace of the ConfigMap. If not specified, defaults to the PlatformMesh resource's namespace.
 	// +optional
 	Namespace string `json:"namespace,omitempty"`
 }

pkg/merge/merge.go (1)

71-75: Remove commented-out dead code.

This commented block should be removed. The logic is correct as-is: when both values are non-objects, dst already contains the correct value since it has higher precedence, so no action is needed. Keeping commented code reduces readability.

🔎 Proposed fix

 		if val != nil {
 			log.Debug().Msgf("keeping object value for %s from destination (destination has higher precedence)", key)
 		}
-		// } else {
-		// 	// Both are non-objects (strings, arrays, numbers, etc.)
-		// 	// dst already has the correct value since it has higher precedence
-		// 	// No action needed - dst value is already in place
 	}

pkg/subroutines/deployment_helpers_test.go (1)

130-134: Type assertion chain could panic on unexpected structure.

The nested type assertion at line 133 assumes a specific structure. While acceptable in tests with controlled data, consider using a safer access pattern for robustness.

🔎 Safer alternative

 	result, err := getOrCreateObject(ctx, clientMock, obj)
 	s.NoError(err)
 	s.NotNil(result)
-	s.Equal("value", result.Object["spec"].(map[string]interface{})["key"])
+	spec, ok := result.Object["spec"].(map[string]interface{})
+	s.Require().True(ok, "spec should be a map")
+	s.Equal("value", spec["key"])

test/e2e/kind/helpers.go (2)

44-67: Consider extracting common logic to reduce duplication.

ApplyFile and ApplyTemplateFromFile share nearly identical logic for iterating objects and applying patches. Similarly, unstructuredsFromFileNoTemplate duplicates most of unstructuredsFromFile. Consider extracting the common YAML parsing and patching logic into shared helpers.

Suggested approach

// Common parsing helper
func parseYAMLObjects(data []byte, log *logger.Logger, source string) ([]unstructured.Unstructured, error) {
    objects := strings.Split(string(data), "---\n")
    var unstructuredObjs []unstructured.Unstructured
    for _, obj := range objects {
        var objMap map[string]interface{}
        if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
            return nil, errors.Wrap(err, "Failed to unmarshal YAML from %s", source)
        }
        log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
        unstructuredObjs = append(unstructuredObjs, unstructured.Unstructured{Object: objMap})
    }
    return unstructuredObjs, nil
}

Also applies to: 100-124

---

56-65: Error accumulation may lose error details.

The errors.Wrap(errRet, ...) pattern only preserves the last error message context. If multiple objects fail to apply, earlier error details may be obscured.

Consider using errors.Join or collecting all errors

-var errRet error = nil
+var errs []error
 for _, obj := range objs {
     if obj.Object == nil {
         continue
     }
     err = k8sClient.Patch(ctx, &obj, client.Apply, client.FieldOwner("platform-mesh-operator"))
     if err != nil {
-        errRet = errors.Wrap(errRet, "Failed to apply manifest file: %s (%s/%s)", path, obj.GetKind(), obj.GetName())
+        errs = append(errs, errors.Wrap(err, "Failed to apply manifest file: %s (%s/%s)", path, obj.GetKind(), obj.GetName()))
     }
 }
-return errRet
+return errors.Join(errs...)

pkg/subroutines/resource/subroutine_test.go (1)

182-183: Unused mock declaration.

managerMock is created on line 182 but only used for GetClient() expectation on line 222, which is never called since the subroutine now directly uses the client. Consider removing the unused mock.

Remove unused managerMock

 for _, tt := range tests {
     s.Run(tt.name, func() {
-        managerMock := new(mocks.Manager)
         clientMock := new(mocks.Client)
         ctx := context.TODO()
         // ...
-        managerMock.On("GetClient").Return(clientMock)
         subroutine := subroutines.NewResourceSubroutine(clientMock)

pkg/subroutines/deployment_test.go (1)

51-51: Unused field assignment.

s.operatorConfig is assigned on line 51 but does not appear to be used elsewhere in the visible test code. If this is intentional for future tests, consider adding a comment; otherwise, remove the assignment.

pkg/subroutines/providersecret.go (2)

253-258: Return value in CreateOrUpdate callback is misleading.

The callback returns err (line 257) which refers to the error from clientcmd.Write. While this works because err would be nil if execution reaches this point, it's confusing. Explicitly return nil for clarity.

Proposed fix

 	_, err = controllerutil.CreateOrUpdate(ctx, r.client, providerSecret, func() error {
 		providerSecret.Data = map[string][]byte{
 			"kubeconfig": kcpConfigBytes,
 		}
-		return err
+		return nil
 	})

---

324-327: Same callback return value issue.

Apply the same fix as suggested for line 257.

Proposed fix

 	_, err = controllerutil.CreateOrUpdate(ctx, r.client, initializerSecret, func() error {
 		initializerSecret.Data = map[string][]byte{"kubeconfig": data}
-		return err
+		return nil
 	})

internal/controller/platformmesh_controller.go (1)

97-99: Consider logging errors in mapping function.

When List fails, an empty slice is returned silently. Adding a log statement would aid debugging in production.

Proposed fix

 	if err := r.client.List(ctx, platformMeshList); err != nil {
+		// Consider using a logger if available in context
 		return requests
 	}

Note: You may need to obtain a logger from context or add one to the reconciler struct.

cmd/operator.go (1)

120-136: Infra client initialization could be streamlined.

A local clientInfra is created (lines 125-129) before checking if RemoteInfra.Enabled. If enabled, the local client is discarded and replaced. Consider restructuring to avoid unnecessary work:

Proposed restructure

 	var clientInfra client.Client
-	restCfgInfra := ctrl.GetConfigOrDie()
-	restCfgInfra.Wrap(func(rt http.RoundTripper) http.RoundTripper {
-		return otelhttp.NewTransport(rt)
-	})
-	clientInfra, err = client.New(restCfgInfra, client.Options{Scheme: subroutines.GetClientScheme()})
-	if err != nil {
-		setupLog.Error(err, "unable to create Infra client")
-		os.Exit(1)
-	}
 	if operatorCfg.RemoteInfra.Enabled {
 		clientInfra, _, err = subroutines.GetClientAndRestConfig(operatorCfg.RemoteInfra.Kubeconfig)
 		if err != nil {
 			setupLog.Error(err, "unable to create Infra client")
 			os.Exit(1)
 		}
+	} else {
+		restCfgInfra := ctrl.GetConfigOrDie()
+		restCfgInfra.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+			return otelhttp.NewTransport(rt)
+		})
+		clientInfra, err = client.New(restCfgInfra, client.Options{Scheme: subroutines.GetClientScheme()})
+		if err != nil {
+			setupLog.Error(err, "unable to create Infra client")
+			os.Exit(1)
+		}
 	}

pkg/subroutines/subroutine_helpers.go (1)

555-576: Consider caching the scheme to avoid repeated allocations.

GetClientScheme() creates a new scheme and registers types on every invocation. If called frequently, this incurs unnecessary allocations. A package-level sync.Once initialization would be more efficient.

Proposed optimization

import "sync"

var (
	clientScheme     *runtime.Scheme
	clientSchemeOnce sync.Once
)

func GetClientScheme() *runtime.Scheme {
	clientSchemeOnce.Do(func() {
		var gvk = schema.GroupVersionKind{
			Group:   "delivery.ocm.software",
			Version: "v1alpha1",
			Kind:    "Resource",
		}

		clientScheme = runtime.NewScheme()
		utilruntime.Must(v1alpha1.AddToScheme(clientScheme))
		utilruntime.Must(helmv2beta.AddToScheme(clientScheme))
		utilruntime.Must(corev1.AddToScheme(clientScheme))
		utilruntime.Must(appsv1.AddToScheme(clientScheme))
		utilruntime.Must(certmanager.AddToScheme(clientScheme))
		utilruntime.Must(fluxcdv1.AddToScheme(clientScheme))
		utilruntime.Must(fluxcdv2.AddToScheme(clientScheme))

		clientScheme.AddKnownTypeWithName(gvk, &unstructured.Unstructured{})
		clientScheme.AddKnownTypeWithName(gvk.GroupVersion().WithKind(gvk.Kind+"List"), &unstructured.UnstructuredList{})
	})
	return clientScheme
}

pkg/subroutines/deployment.go (5)

106-109: Error returned without context.

When the ConfigMap doesn't exist, the raw Kubernetes error is returned without additional context indicating which ConfigMap was being fetched.

Proposed fix

-	return nil, err
+	return nil, errors.Wrap(err, "failed to get profile ConfigMap %s/%s", configMapNamespace, configMapName)

---

316-319: Unnecessary logger creation.

A new logger is instantiated here, but a context-based logger should be available from ctx. This is inconsistent with other methods that use logger.LoadLoggerFromContext(ctx).

Proposed fix

-	log, err := logger.New(logger.DefaultConfig())
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed to create logger")
-	}
+	log := logger.LoadLoggerFromContext(ctx)
 	tmplVars, err := merge.MergeMaps(infraProfileMap, templateVarsMap, log)

---

462-465: Inconsistent logger instantiation.

Same issue as templateVarsFromProfileInfra: creating a new logger instead of using the context logger.

Proposed fix

-	log, err := logger.New(logger.DefaultConfig())
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed to create logger")
-	}
+	log := logger.LoadLoggerFromContext(ctx)

---

636-656: Template parsing/execution failures are silently ignored.

When template parsing (line 643) or execution (line 648) fails, the original value is returned without any logging. This could mask configuration errors that are difficult to debug.

Consider adding debug-level logging when templates fail to parse or execute.

---

684-832: Significant code duplication between infra and runtime component renderers.

renderAndApplyComponentsInfraTemplates and renderAndApplyComponentsRuntimeTemplates share nearly identical logic, differing only in the directory path, client, and log messages. Consider extracting a common helper method.

Example refactor

func (r *DeploymentSubroutine) renderAndApplyComponentTemplates(
	ctx context.Context,
	inst *v1alpha1.PlatformMesh,
	templateVars apiextensionsv1.JSON,
	subDir string,
	k8sClient client.Client,
	targetName string,
) errors.OperatorError {
	log := logger.LoadLoggerFromContext(ctx).ChildLogger("subroutine", r.GetName())

	tmplVars, err := r.buildComponentsTemplateVars(ctx, inst, templateVars)
	if err != nil {
		log.Error().Err(err).Msgf("Failed to build components template data for %s", targetName)
		return errors.NewOperatorError(err, true, true)
	}

	err = filepath.WalkDir(r.gotemplatesComponentsDir+"/"+subDir, func(path string, d fs.DirEntry, err error) error {
		// ... shared logic ...
	})

	if err != nil {
		log.Error().Err(err).Msgf("Failed to render and apply components %s templates", targetName)
		return errors.NewOperatorError(err, false, true)
	}

	return nil
}

📜 Review details

Configuration used: Repository: platform-mesh/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b64a99a and bebe8ea.

⛔ Files ignored due to path filters (3)

• api/v1alpha1/zz_generated.deepcopy.go is excluded by !**/*_generated.*
• config/crd/core.platform-mesh.io_platformmeshes.yaml is excluded by !config/crd/**.yaml
• config/rbac/role.yaml is excluded by !config/rbac/**.yaml

📒 Files selected for processing (51)

• .gitignore
• Dockerfile
• README.md
• Taskfile.yml
• api/v1alpha1/platformmesh_types.go
• cmd/operator.go
• gotemplates/components/infra/helmreleases.yaml
• gotemplates/components/runtime/ocm-image-resources.yaml
• gotemplates/components/runtime/ocm-resources.yaml
• gotemplates/infra/infra/cert-manager/helmrelease.yaml
• gotemplates/infra/infra/gateway-api/kustomization.yaml
• gotemplates/infra/infra/traefik/helmrelease-crds.yaml
• gotemplates/infra/infra/traefik/helmrelease.yaml
• gotemplates/infra/runtime/cert-manager/resource.yaml
• gotemplates/infra/runtime/gateway-api/resource.yaml
• gotemplates/infra/runtime/traefik/resource-crds.yaml
• gotemplates/infra/runtime/traefik/resource.yaml
• internal/config/config.go
• internal/controller/platformmesh_controller.go
• internal/controller/realm_controller.go
• internal/controller/resource_controller.go
• manifests/k8s/platform-mesh-operator-components/release.yaml
• manifests/k8s/platform-mesh-operator-components/resource.yaml
• manifests/k8s/platform-mesh-operator-infra-components/release.yaml
• manifests/k8s/platform-mesh-operator-infra-components/resource.yaml
• pkg/merge/merge.go
• pkg/subroutines/defaults.go
• pkg/subroutines/deployment.go
• pkg/subroutines/deployment_helpers.go
• pkg/subroutines/deployment_helpers_test.go
• pkg/subroutines/deployment_test.go
• pkg/subroutines/featuretoggles.go
• pkg/subroutines/featuretoggles_test.go
• pkg/subroutines/kcpsetup.go
• pkg/subroutines/kcpsetup_test.go
• pkg/subroutines/providersecret.go
• pkg/subroutines/providersecret_test.go
• pkg/subroutines/resource/subroutine.go
• pkg/subroutines/resource/subroutine_test.go
• pkg/subroutines/subroutine_helpers.go
• pkg/subroutines/subroutine_testexports.go
• pkg/subroutines/wait_test.go
• test/e2e/kind/helpers.go
• test/e2e/kind/kustomize/base/crds/delivery.ocm.software_replications.yaml
• test/e2e/kind/kustomize/base/crds/kustomization.yaml
• test/e2e/kind/kustomize/base/ocm-k8s-toolkit/flux-sync.yaml
• test/e2e/kind/kustomize/components/ocm/component.yaml
• test/e2e/kind/kustomize/components/ocm/repository.yaml
• test/e2e/kind/suite_kind_test.go
• test/e2e/kind/yaml/platform-mesh-resource/default-profile.yaml
• test/e2e/kind/yaml/platform-mesh-resource/platform-mesh.yaml

💤 Files with no reviewable changes (5)

• manifests/k8s/platform-mesh-operator-components/resource.yaml
• manifests/k8s/platform-mesh-operator-infra-components/resource.yaml
• manifests/k8s/platform-mesh-operator-infra-components/release.yaml
• pkg/subroutines/subroutine_testexports.go
• manifests/k8s/platform-mesh-operator-components/release.yaml

🧰 Additional context used

🧬 Code graph analysis (14)

internal/controller/realm_controller.go (1)

pkg/subroutines/subroutine_helpers.go (1)

• GetClientAndRestConfig (516-553)

pkg/subroutines/resource/subroutine_test.go (2)

pkg/subroutines/resource/subroutine.go (1)

• NewResourceSubroutine (50-52)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/resource/subroutine.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/kcpsetup_test.go (5)

pkg/subroutines/kcpsetup.go (1)

• NewKcpsetupSubroutine (49-58)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

pkg/subroutines/mocks/mock_KcpHelper.go (1)

• KcpHelper (13-15)

pkg/subroutines/deployment_helpers_test.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/subroutine_helpers.go (2)

api/v1alpha1/platformmesh_types.go (1)

• PlatformMesh (184-190)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/featuretoggles.go (3)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/deployment_test.go (4)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

pkg/subroutines/deployment.go (2)

• DeploymentSubroutine (39-47)
• NewDeploymentSubroutine (54-71)

internal/config/config.go (1)

• OperatorConfig (4-53)

cmd/operator.go (4)

pkg/subroutines/subroutine_helpers.go (2)

• GetClientAndRestConfig (516-553)
• GetClientScheme (555-576)

internal/controller/platformmesh_controller.go (1)

• NewPlatformMeshReconciler (129-158)

internal/controller/resource_controller.go (1)

• NewResourceReconciler (78-98)

internal/controller/realm_controller.go (1)

• NewRealmReconciler (28-63)

test/e2e/kind/helpers.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/providersecret.go (2)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

test/e2e/kind/suite_kind_test.go (3)

test/e2e/kind/helpers.go (2)

• ApplyTemplateFromFile (20-42)
• ApplyFile (45-67)

internal/controller/platformmesh_controller.go (1)

• NewPlatformMeshReconciler (129-158)

internal/controller/resource_controller.go (1)

• NewResourceReconciler (78-98)

pkg/subroutines/deployment.go (3)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/subroutine_helpers.go (2)

• TemplateVars (258-283)
• GetSecret (82-92)

pkg/merge/merge.go (1)

• MergeMaps (9-42)

pkg/subroutines/kcpsetup.go (2)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

internal/config/config.go (1)

• OperatorConfig (4-53)

🪛 YAMLlint (1.37.1)

gotemplates/infra/runtime/cert-manager/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/runtime/traefik/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/runtime/gateway-api/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/runtime/traefik/resource-crds.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/components/runtime/ocm-image-resources.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/components/runtime/ocm-resources.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/infra/infra/gateway-api/kustomization.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/infra/cert-manager/helmrelease.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/components/infra/helmreleases.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/infra/infra/traefik/helmrelease-crds.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/infra/traefik/helmrelease.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (81)

.gitignore (1)

45-47: Approved: Additions align with PR scope.

The three new ignore patterns appropriately support the deployment and template rendering workflows introduced in this PR. The patterns will prevent generated kubeconfig files, debug artifacts, and editor-specific temporary files from being tracked.

Minor note on pattern specificity: Line 46 uses debug which matches any file or directory named "debug" at any level in the repository tree. If the intent is to ignore only a top-level debug directory or only debug directories (not files), consider using /debug or debug/ respectively. If broad matching of all debug artifacts is intentional, the current pattern is suitable.

gotemplates/infra/runtime/gateway-api/resource.yaml (2)

1-1: YAMLlint false positive (can be ignored).

The YAMLlint syntax error on line 1 is expected and can be safely ignored. The {{- if delimiter is valid Go template syntax, which YAMLlint does not recognize.

---

9-9: Namespace variable usage is intentional and consistent.

Line 9 uses .helmReleaseNamespace and line 18 uses .releaseNamespace. This pattern is consistent across all similar OCM Resource templates and is intentional: both variables are populated with the same namespace value in the Go template rendering code (see pkg/subroutines/deployment.go). The different variable names serve to distinguish between contexts (Helm metadata vs. OCM Repository references) but resolve to identical values. No correction needed.

Taskfile.yml (2)

20-20: LGTM: Centralized test timeout configuration.

The introduction of the TEST_TIMEOUT variable improves maintainability by centralizing timeout configuration. The 15-minute default is appropriate for the expanded test suite that includes template rendering and dual-cluster operations.

---

75-75: LGTM: Consistent timeout variable usage.

All test commands correctly reference the TEST_TIMEOUT variable, ensuring consistent timeout behavior across different test execution paths.

Also applies to: 87-87, 105-105

gotemplates/infra/infra/traefik/helmrelease-crds.yaml (2)

16-23: LGTM: Chart source configuration is correct.

The chart spec correctly references the HelmRepository with appropriate reconciliation strategy.

---

1-29: Remove this suggestion; values block is not required for CRD HelmRelease templates.

The test configuration for traefikCRDs does not include a values field, which is consistent with Helm charts containing only Custom Resource Definitions. CRD charts are definition-only resources that do not require configuration values, unlike the main Traefik release or cert-manager release which require custom configuration. The template is correct as written.

Likely an incorrect or invalid review comment.

gotemplates/infra/runtime/traefik/resource-crds.yaml (2)

10-13: LGTM: Resource spec structure is correct.

The componentRef, resource.byReference with dynamic referencePath, and skipVerify configuration are properly structured for OCM resource resolution.

Also applies to: 19-28

---

1-29: Namespace pattern is consistent and intentional across all Resource templates.

The namespace difference is not a bug. All Resource templates under gotemplates/infra/runtime/ (traefik/resource.yaml, traefik/resource-crds.yaml, gateway-api/resource.yaml, and cert-manager/resource.yaml) consistently use .helmReleaseNamespace for metadata.namespace and .releaseNamespace for the ocmConfig Repository reference. This pattern is replicated identically across all four components, indicating this is an intentional design where the two namespaces serve different purposes.

Likely an incorrect or invalid review comment.

gotemplates/infra/runtime/traefik/resource.yaml (1)

1-29: No action required. The namespace difference between metadata.namespace (.helmReleaseNamespace) and ocmConfig Repository namespace (.releaseNamespace) is intentional and consistent across all Resource templates in the infra/runtime directory. This pattern correctly implements cross-namespace resource references, where the Resource object itself is deployed in the helm release namespace while referencing a Repository in a separate namespace.

Likely an incorrect or invalid review comment.

api/v1alpha1/platformmesh_types.go (1)

176-180: PrintColumn updates align with new subroutine additions.

The new FEATURES and WAIT columns correctly reference their respective subroutine condition types. The column configuration follows the existing pattern.

pkg/subroutines/kcpsetup_test.go (1)

53-53: Constructor calls correctly updated to match new signature.

All invocations of NewKcpsetupSubroutine have been updated to remove the URL parameter, consistent with the signature change in pkg/subroutines/kcpsetup.go which now accepts (client, helper, cfg, kcpdir).

pkg/merge/merge.go (1)

44-78: Merge logic correctly implements destination precedence.

The refactored mergeObject function properly handles:

1. Adding new keys from src when absent in dst
2. Recursive merging when both are objects
3. Preserving dst values on type mismatches with appropriate debug logging

gotemplates/components/runtime/ocm-resources.yaml (2)

33-40: Verify referencePath logic for unintended duplication.

When $config.referencePath is not set, the template uses $values.ocm.referencePath (lines 36-38) and then unconditionally appends - name: {{ $service }} (line 39). If the global referencePath already includes service-specific entries or expects the service name to be appended elsewhere, this could cause duplication.

Confirm this is the intended behavior. If the service name should only be appended when using the fallback path, consider adjusting the template structure.

---

1-53: Go template for OCM resources is correctly structured.

The static analysis warning about YAML syntax is a false positive—this is a Go template file that generates valid YAML when rendered. The template properly iterates over enabled services and generates Resource manifests with appropriate conditional logic for labels and configuration fallbacks.

test/e2e/kind/yaml/platform-mesh-resource/platform-mesh.yaml (1)

7-13: ProfileConfigMap and exposure configuration properly added.

The new profileConfigMap reference correctly points to the profile ConfigMap, and the exposure section includes the required baseDomain and protocol fields for template rendering.

test/e2e/kind/yaml/platform-mesh-resource/default-profile.yaml (2)

275-275: Inconsistent placeholder usage: baseDomain vs baseDomainPort.

This line uses {{ .baseDomain }} while similar URLs elsewhere use {{ .baseDomainPort }}. Verify whether the port should be included here for consistency with other API URLs.

Compare with line 279-281 which use {{ .baseDomainPort }}.

---

1-392: Profile ConfigMap provides comprehensive default configuration.

The ConfigMap structure correctly embeds the profile.yaml with all necessary service configurations, OCM settings, and infrastructure components. The template placeholders enable environment-specific customization.

pkg/subroutines/deployment_helpers_test.go (2)

38-103: Table-driven tests for metadata updates are well-structured.

The tests cover key scenarios: updating labels/annotations and handling missing metadata. The validation functions properly verify expected outcomes.

---

136-199: SSA apply and error handling paths are properly tested.

Tests correctly verify:

• NotFound triggers SSA Patch with client.Apply and FieldOwner
• Generic Get errors are properly propagated with descriptive messages

pkg/subroutines/deployment_helpers.go (3)

34-42: LGTM!

The metadata synchronization logic correctly handles nil maps and updates only when labels or annotations are provided.

---

162-218: LGTM!

The template function map provides comprehensive Helm-like template functions with correct implementations. The nindent function properly handles edge cases including empty strings, all-empty-line inputs, and mixed content.

---

67-113: No issues identified. The applyManifest method is properly implemented as a method on DeploymentSubroutine at line 839 in pkg/subroutines/deployment.go and is correctly invoked at line 99 in the reviewed code.

test/e2e/kind/kustomize/components/ocm/repository.yaml (1)

5-5: LGTM!

The namespace change from default to platform-mesh-system aligns with the PR's standardization of operational namespace for Platform Mesh resources.

test/e2e/kind/kustomize/base/crds/kustomization.yaml (1)

7-7: LGTM!

The addition of the Replication CRD resource appropriately extends the Kustomization resources to support OCM component replication functionality.

pkg/subroutines/featuretoggles_test.go (1)

45-47: LGTM!

The removal of the kcpUrl parameter from the NewFeatureToggleSubroutine constructor aligns with the PR's refactoring to eliminate unused KCP URL configuration.

gotemplates/infra/infra/gateway-api/kustomization.yaml (1)

1-23: LGTM! (YAMLlint false positive can be ignored)

The template correctly uses Go template syntax for conditional rendering. The YAMLlint error is a false positive because the tool does not recognize Go template directives ({{- if }}). When rendered with appropriate template variables, this will produce valid YAML.

gotemplates/infra/infra/cert-manager/helmrelease.yaml (1)

1-26: LGTM: Template structure is correct.

The HelmRelease template follows FluxCD v2 conventions correctly. The use of OCIRepository for chart source, conditional kubeConfig block, and operator-created label align with the profile-driven deployment approach introduced in this PR.

Note: The YAMLlint syntax error is a false positive—the {{- syntax is valid Go template syntax, not malformed YAML.

test/e2e/kind/kustomize/components/ocm/component.yaml (1)

5-5: LGTM: Namespace consolidation is consistent.

The namespace changes from default to platform-mesh-system align with the broader namespace consolidation introduced in this PR and are consistent with updates in other files, including the wait configuration in pkg/subroutines/defaults.go.

Also applies to: 14-14

test/e2e/kind/kustomize/base/ocm-k8s-toolkit/flux-sync.yaml (1)

39-48: LGTM: RBAC permissions for OCM replications.

The ClusterRole patch appropriately grants permissions for managing OCM replication resources, including status and finalizers subresources. The permission scope follows standard controller patterns.

pkg/subroutines/defaults.go (1)

97-97: LGTM: Wait configuration updated for unified profile approach.

The wait configuration correctly reflects the transition from two separate Helm charts to a unified profile-based deployment. The label-based selector (core.platform-mesh.io/operator-created: "true") provides flexibility to wait for all operator-created HelmReleases rather than specific named releases. The namespace consolidation to platform-mesh-system aligns with changes across the codebase.

Also applies to: 101-103

gotemplates/infra/infra/traefik/helmrelease.yaml (1)

1-32: LGTM: Template structure is correct.

The HelmRelease template follows FluxCD v2 conventions correctly. The use of chart.spec with HelmRepository source (appropriate for Helm repositories vs. OCI registries), the dependsOn block ensuring CRDs are installed first, and the operator-created label all align with best practices and the unified profile approach.

Note: The YAMLlint syntax error is a false positive—the {{- syntax is valid Go template syntax.

pkg/subroutines/featuretoggles.go (2)

43-50: LGTM! Constructor signature simplification aligns with remote deployment architecture.

The removal of the kcpUrl parameter and the shift to dynamic host resolution via getExternalKcpHost at line 131 is consistent with the PR's objective to support flexible remote cluster configurations.

---

131-135: LGTM! Dynamic KCP host resolution implemented correctly.

The use of getExternalKcpHost(inst, &operatorCfg) replaces the hardcoded URL approach, enabling flexible external endpoint configuration for remote deployments.

gotemplates/infra/runtime/cert-manager/resource.yaml (1)

1-29: LGTM! OCM Resource template is well-structured.

The template correctly implements conditional rendering and proper iteration over reference paths. The YAMLlint syntax error is a false positive—this file contains Go template directives, not pure YAML.

internal/controller/realm_controller.go (1)

29-38: LGTM! Remote infra client initialization is correctly implemented.

The conditional remote client setup with proper error handling and logging aligns with the PR's remote deployment objectives.

internal/controller/resource_controller.go (1)

78-86: LGTM! Client parameter addition with defensive nil check.

The addition of the clientInfra parameter with a nil check and helpful comment implements good defensive programming practices. The pattern is consistent with other reconcilers in this PR.

gotemplates/components/infra/helmreleases.yaml (1)

1-54: LGTM! HelmRelease template structure is well-designed.

The template correctly implements conditional chart source logic (HelmRepository, GitRepository, or OCIRepository) with proper iteration over services. The structure supports the profile-driven configuration approach described in the PR objectives. The YAMLlint syntax error is a false positive for Go template files.

internal/config/config.go (2)

36-41: Verify the nested Resource struct with squash tag behavior.

The Resource struct within Wait uses mapstructure:",squash", which means subroutines-resource-enabled will be parsed at the same level as subroutines-wait-enabled. This may cause confusion as the key names don't reflect the hierarchy. Ensure this is intentional and documented.

---

43-52: Remote infrastructure and runtime configuration structure looks correct.

The RemoteInfra and RemoteRuntime configuration groups provide the necessary fields for dual-cluster operation. Default kubeconfig paths and secret references are appropriately defined.

pkg/subroutines/providersecret_test.go (3)

80-87: Test configuration setup is appropriate.

The test suite now properly initializes the operator configuration with KCP-related fields, enabling config-driven test scenarios.

---

1642-1647: Additional test coverage for internal-kubeconfig provider connection.

The new test case covers the internal kubeconfig path, ensuring both external and internal provider connections are validated.

---

1761-1785: Mock expectation for internal-kubeconfig URL rewrite.

The test verifies that internal kubeconfig URLs are correctly rewritten to use the front-proxy service endpoint. The expected URL format aligns with the internal cluster service naming convention.

test/e2e/kind/helpers.go (1)

18-42: Function rename provides clarity.

Renaming ApplyManifestFromFile to ApplyTemplateFromFile clearly communicates that template variable replacement occurs.

pkg/subroutines/kcpsetup.go (3)

46-47: Dedicated field manager constant for KCP setup patches.

Using fieldManagerKcpSetup instead of a generic field owner improves Server-Side Apply ownership tracking and prevents field conflicts with other controllers.

---

49-58: Constructor signature simplified by removing kcpUrl parameter.

The kcpUrl is now resolved dynamically via getExternalKcpHost, reducing constructor complexity and ensuring URL consistency with the instance and config state.

---

99-104: Centralized KCP host resolution is implemented correctly.

The getExternalKcpHost function properly centralizes external KCP URL construction logic and is consistently used across the codebase (in kcpsetup.go, providersecret.go, and featuretoggles.go), ensuring uniform handling of both front-proxy and custom domain configurations.

pkg/subroutines/resource/subroutine_test.go (3)

26-29: Test setup updated for new constructor signature.

The SetupTest now correctly uses NewResourceSubroutine(new(mocks.Client)), reflecting the constructor change from Manager to Client.

---

225-237: Test expectations correctly updated for Server-Side Apply.

The test now expects Patch with 5 arguments (ctx, obj, patch, opts...) instead of the previous Get+Update pattern, aligning with the SSA implementation change.

---

433-436: Verify expected call counts are correct.

The comment states "updateHelmRepository: 1 Get + 1 Update" and "updateHelmRelease: 1 Patch (no Get, no Update)". Ensure these counts match the actual implementation behavior for comprehensive test coverage.

pkg/subroutines/wait_test.go (3)

62-68: Test expectations simplified for single List call.

The mock now expects a single List call instead of multiple, reflecting the streamlined wait logic that queries resources using a label selector.

---

99-103: Test resource metadata updated for new conventions.

The namespace changed to platform-mesh-system and the label key changed to core.platform-mesh.io/operator-created with value "true", aligning with the updated operator resource management pattern.

---

361-372: Test data updated to use generic test resource name.

Using test-helmrelease instead of a specific component name makes the test more maintainable and independent of actual deployment resource naming.

pkg/subroutines/deployment_test.go (2)

19-24: Test suite expanded with operator configuration.

The suite now includes operatorConfig field to support configuration-driven test scenarios for the deployment subroutine.

---

40-53: Test file lacks test methods to verify the configuration.

The test file contains only setup code (54 lines total) with no actual test methods. While the DeploymentSubroutine is initialized with nil for clientInfra and RemoteInfra.Enabled = true, no tests execute to verify this configuration works correctly. The Reconcile method, which unconditionally uses clientInfra at line 194 (in getHelmRelease), is never called. Add test methods to exercise the actual behavior.

test/e2e/kind/kustomize/base/crds/delivery.ocm.software_replications.yaml (1)

1-335: CRD structure is well-defined.

The Replication CRD follows Kubernetes conventions with appropriate OpenAPI v3 schema validations. The x-kubernetes-validations rules correctly enforce allowed apiVersion/kind combinations for OCM configuration references.

pkg/subroutines/providersecret.go (1)

46-66: Constructor and struct refactoring look correct.

The transition from kcpUrl string to cfgOperator *config.OperatorConfig is properly implemented.

pkg/subroutines/resource/subroutine.go (3)

44-52: Server-Side Apply implementation is appropriate.

The introduction of resourceFieldManager and the transition to SSA with client.Patch ensures proper field ownership when multiple controllers manage the same resources. This prevents field conflicts with other subroutines.

---

172-196: SSA patch implementation for image tag updates is correct.

The minimal patch object approach ensures only the managed field is tracked, and ForceOwnership handles conflicts appropriately.

---

364-369: Timeout field addition is appropriate.

Setting a 5-minute timeout for GitRepository resources is reasonable. Note that line 369 returns err which will be nil at this point—returning nil explicitly would be marginally clearer but this is a minor nit.

test/e2e/kind/suite_kind_test.go (3)

335-336: Test helper migration to ApplyTemplateFromFile is correct.

The switch to template-aware helpers aligns with the broader PR changes. Passing an empty map when no template variables are needed is acceptable.

---

419-426: Profile and PlatformMesh resource application is correctly implemented.

Using ApplyFile for static manifests without template variables is the right approach.

---

543-554: Reconciler initialization with client parameter is correct.

Passing mgr.GetClient() as clientInfra in tests where remote mode is disabled (lines 522-523) ensures consistent behavior with local-only deployments.

README.md (2)

53-156: Configuration documentation is comprehensive and well-organized.

The tables cover all configuration options with clear descriptions and defaults. The notes section clarifies configuration methods and in-cluster behavior.

---

258-307: Profile Configuration documentation is clear and informative.

The section explains the default profile behavior, custom profile configuration, and unified structure. The example YAML demonstrates the expected format.

internal/controller/platformmesh_controller.go (2)

78-85: ConfigMap watch implementation enables reactive profile updates.

This correctly triggers PlatformMesh reconciliation when referenced ConfigMaps change, supporting dynamic profile configuration.

---

129-157: Constructor properly wires clients to subroutines.

The clientInfra parameter is correctly passed to subroutines that need to operate on the infrastructure cluster (DeploymentSubroutine, WaitSubroutine), while other subroutines receive the manager client as appropriate.

cmd/operator.go (2)

90-98: Remote runtime configuration handling is correct.

The conditional replacement of restCfg when RemoteRuntime.Enabled is true allows the operator to reconcile PlatformMesh resources on a remote cluster while the operator itself runs locally.

---

137-154: Reconciler setup with scoped loggers improves observability.

Using log.ChildLogger("type", "...") for each reconciler provides clear attribution in logs, aiding debugging in multi-controller environments.

pkg/subroutines/subroutine_helpers.go (2)

182-230: LGTM.

The updated signature and injection of remote runtime configuration fields align with the new dual-cluster deployment model.

---

578-588: LGTM.

The host resolution helpers correctly derive KCP URLs based on instance exposure settings or operator configuration defaults.

pkg/subroutines/deployment.go (10)

39-71: LGTM.

The refactored struct and constructor properly support the dual-client architecture with separate infra and runtime clients.

---

111-153: LGTM.

Profile section loading correctly validates the presence of required infra and components sections in the ConfigMap.

---

333-457: LGTM.

The function correctly orchestrates merging of infra profile, templateVars, PlatformMesh.spec.Values, OCM config, and profile-components services into a unified template variable map.

---

167-280: LGTM.

The Process method correctly orchestrates the deployment flow, using the appropriate client (infra vs runtime) for each operation and handling readiness checks before proceeding.

---

414-414: > Likely an incorrect or invalid review comment.

---

846-904: Verify constant definitions.

The code references fieldManagerDeployment, kindHelmRelease, kindResource, and specFieldInterval, but these constants are not defined in the provided code snippet. Confirm these constants are properly defined elsewhere in the pkg/subroutines/deployment.go file or imported from another package.

---

886-916: I cannot access the repository to verify the claims in this review comment. The repository failed to clone, preventing me from examining:

• The actual retry logic implementation for HelmRelease (lines 861-883)
• Whether Resource and generic objects genuinely lack retry logic
• The concurrent modification scenarios and whether SSA conflicts are realistic for these object types
• How getOrCreateObject behaves and its pointer semantics

To properly evaluate this comment, manual verification is needed by reviewing the complete applyManifest function (lines 834-917+), examining the HelmRelease section for its retry pattern, and assessing whether Resource and generic objects encounter similar transient failures during SSA operations.

---

668-682: The renderAndApplyTemplates method is defined in pkg/subroutines/deployment_helpers.go and properly handles errors by wrapping them with context before returning as OperatorError. The method is correctly invoked on lines 668 and 681. No issues found.

Likely an incorrect or invalid review comment.

---

1128-1135: No issues identified. The exported method ApplyManifestFromFileWithMergedValues is properly defined on *DeploymentSubroutine in subroutine_testexports.go and correctly delegates to the unexported implementation function applyManifestFromFileWithMergedValues at line 1150. The method call at line 1128 is valid.

Likely an incorrect or invalid review comment.

---

840-843: No action required — pointer semantics are intentional.

The getOrCreateObject function is defined in pkg/subroutines/deployment_helpers.go (not invisible). The pointer comparison on line 847 (existing == obj) is intentionally correct: when an object is created, the function returns the input obj pointer; when an object exists, it returns a different pointer to the retrieved object. This design allows the calling code to detect whether the object was just created.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

pkg/subroutines/defaults.go (1)

87-111: Parameterize namespace in wait configuration to match the PlatformMesh instance namespace.

The wait configuration hardcodes "platform-mesh-system" as the namespace, but HelmReleases are created in the PlatformMesh instance's namespace (as seen in deployment.go lines 307, 327, 448-449). This creates a critical mismatch: if a PlatformMesh resource exists in a namespace other than "platform-mesh-system", the wait subroutine will not find the HelmReleases it expects because it searches in the wrong namespace.

The label "core.platform-mesh.io/operator-created=true" is consistently applied across all HelmRelease templates and correctly validated by the wait configuration.

Update DEFAULT_WAIT_CONFIG to use the PlatformMesh instance's namespace instead of the hardcoded value, aligning with the actual resource creation behavior.

internal/controller/resource_controller.go (1)

78-90: Reconcile inconsistent client usage patterns across reconcilers.

The NewResourceReconciler passes clientInfra to ResourceSubroutine (line 86) but uses mgr.GetClient() for the LifecycleManager (line 90). This pattern is consistent with PlatformMeshReconciler, but contradicts RealmReconciler, which passes clientInfra to its lifecycle manager instead. Align the client usage strategy across all three reconcilers.

♻️ Duplicate comments (8)

Dockerfile (1)

24-24: Builder stage COPY of gotemplates is unnecessary.

The gotemplates directory is loaded at runtime and does not need to be present during the Go build process. The COPY at line 24 in the builder stage duplicates the essential runtime COPY at line 37. Remove line 24 to optimize the build cache.

pkg/subroutines/deployment_helpers.go (3)

28-28: Remove unused constant.

The specFieldInterval constant is defined but not used anywhere in this file.

🔎 Proposed fix

-	// Spec field names
-	specFieldInterval = "interval"
-

---

56-63: Consider refreshing the object after creation.

After SSA Apply via Patch() on line 59, the function returns the input obj on line 62. The controller-runtime client does not mutate the input object in-place with server-generated fields like resourceVersion, uid, or creationTimestamp. While current usage patterns may handle this appropriately, a more robust approach would be to retrieve the created object via Get() to ensure complete server-side metadata is available.

---

139-142: Avoid logging potentially sensitive rendered content.

The error message on line 141 includes the entire rendered YAML output (renderedStr). If the template contains sensitive data such as secrets, tokens, or credentials, this information will be exposed in error logs.

🔎 Proposed fix

 	var objMap map[string]interface{}
 	if err := yaml.Unmarshal(rendered.Bytes(), &objMap); err != nil {
-		return nil, errors.Wrap(err, "Failed to unmarshal rendered YAML. Output:\n%s", renderedStr)
+		return nil, errors.Wrap(err, "Failed to unmarshal rendered YAML from template: %s", path)
 	}

gotemplates/components/runtime/ocm-image-resources.yaml (1)

23-41: Add missing skipVerify field to the spec section.

The spec is missing the skipVerify field, which is present in the sibling template gotemplates/components/runtime/ocm-resources.yaml. For consistency and to properly propagate the verification setting from values configuration, add skipVerify: {{ $values.ocm.skipVerify }} to the spec.

🔎 Proposed fix

 spec:
   componentRef:
     name: {{ $values.ocm.component.name }}
   resource:
     byReference:
       resource:
         name: {{ .resource | default "image"}}
       referencePath:
         {{- if .referencePath }}
         {{- toYaml .referencePath | nindent 8 }}
         {{- else if $config.referencePath }}
         {{- toYaml $config.referencePath | nindent 8 }}
         {{- else }}
         {{- with $values.ocm.referencePath }}
         {{- toYaml . | nindent 8 }}
         {{- end }}
         - name: {{ $service}}
         {{- end }}
+  skipVerify: {{ $values.ocm.skipVerify }}
   interval: 1m
   ocmConfig:

test/e2e/kind/yaml/platform-mesh-resource/default-profile.yaml (1)

170-172: Typo in secret name: "grapqhl" should be "graphql".

The secretName value contains a misspelling that will cause kubeconfig lookup failures.

test/e2e/kind/yaml/platform-mesh-resource/platform-mesh.yaml (1)

155-157: Typo in secret name: "grapqhl" should be "graphql".

The secretName value contains a misspelling that will cause kubeconfig lookup failures at runtime.

pkg/subroutines/subroutine_helpers.go (1)

516-553: Duplicate: log.Fatal() terminates process, making error returns unreachable.

This issue was flagged in a previous review. The log.Fatal() calls invoke os.Exit(1), so the subsequent return nil, nil, err statements are never executed. Replace with log.Error() to allow proper error propagation.

🧹 Nitpick comments (16)

.gitignore (1)

45-47: Consider using trailing slash for directory pattern consistency.

The .cursor/* pattern uses glob matching within the directory, whereas the existing pattern at line 39 (test/e2e/kind/certs/) uses a trailing slash to explicitly match the directory. For consistency and clarity, consider using .cursor/ instead, which more explicitly targets the directory and its contents.

This is a minor style preference and does not affect functionality.

pkg/merge/merge.go (1)

66-75: Remove commented-out code.

The commented-out code at lines 71-74 should be removed. While the explanation is helpful during development, commented code reduces maintainability. Version control preserves the history if this logic is needed in the future.

🔎 Proposed cleanup

 		} else if isObject(dv) {
 			// dst is object but src is not, keep dst (dst has higher precedence)
 			if val != nil {
 				log.Debug().Msgf("keeping object value for %s from destination (destination has higher precedence)", key)
 			}
-			// } else {
-			// 	// Both are non-objects (strings, arrays, numbers, etc.)
-			// 	// dst already has the correct value since it has higher precedence
-			// 	// No action needed - dst value is already in place
 		}
 	}
 	return dst

gotemplates/infra/runtime/traefik/resource.yaml (1)

1-29: Template structure is correct; static analysis warning is a false positive.

The YAMLlint error flagging line 1 is a false positive. The {{- if syntax is valid Go template notation and will be processed before YAML parsing.

The template logic correctly:

• Conditionally renders based on .traefik.enabled
• Iterates over .ocm.referencePath to build the reference list
• Constructs the Resource manifest with proper field interpolation

Consider adding a trailing newline to the file (after line 29), as this is a common convention expected by some tools and linters.

gotemplates/infra/infra/traefik/helmrelease-crds.yaml (1)

1-29: Template structure is correct; static analysis warning is a false positive.

The YAMLlint error on line 1 is a false positive due to Go template syntax. The template correctly:

• Conditionally renders the HelmRelease when .traefikCRDs.enabled is true
• Includes optional kubeConfig block based on .kubeConfigEnabled
• Defines proper FluxCD HelmRelease specification with chart sourceRef and intervals

Consider adding a trailing newline after line 29, consistent with common file conventions.

gotemplates/infra/runtime/traefik/resource-crds.yaml (1)

1-29: Template structure is correct; static analysis warning is a false positive.

The YAMLlint error on line 1 is a false positive due to Go template syntax. The template correctly:

• Conditionally renders when .traefikCRDs.enabled is true
• Constructs the referencePath by iterating over .ocm.referencePath and appending .traefikCRDs.ocmComponentName
• References the resource using .traefikCRDs.ocmResourceName

The pattern mirrors resource.yaml with CRD-specific field references, maintaining consistency across templates.

Consider adding a trailing newline after line 29 for consistency with common file conventions.

gotemplates/components/infra/helmreleases.yaml (1)

18-41: Prefer else if and else for mutually exclusive chart source logic.

The three chart source branches (helmRepo, gitRepo, OCIRepository) are mutually exclusive but use separate {{- if }} blocks. Using {{- else if }} and {{- else }} would make this mutual exclusivity explicit and prevent potential double-rendering if the logic is modified incorrectly in the future.

🔎 Proposed refactor

-  {{- if $config.helmRepo }}
+  {{- if $config.helmRepo }}
   chart:
     spec:
       chart: {{ $config.chart | default $service }}
       reconcileStrategy: ChartVersion
       sourceRef:
         kind: HelmRepository
         name: {{ $service }}
-  {{- end }}
-  {{- if $config.gitRepo }}
+  {{- else if $config.gitRepo }}
   chart:
     spec:
       chart: {{ $config.path | default $service }}
       reconcileStrategy: ChartVersion
       sourceRef:
         kind: GitRepository
         name: {{ $service }}
-  {{- end }}
-  {{- if not (or $config.helmRepo $config.gitRepo) }}
+  {{- else }}
   chartRef:
     kind: OCIRepository
     name: {{ $service }}
     namespace: {{ $.releaseNamespace }}
   {{- end }}

gotemplates/components/runtime/ocm-image-resources.yaml (1)

30-40: Consider documenting the referencePath fallback behavior.

The template implements a three-tier fallback for referencePath: per-resource → per-service → global (with service name appended). While the logic appears correct, the conditional appending of the service name only for the global path may not be immediately obvious to users configuring profiles.

Consider adding a comment in the template explaining this behavior:

       referencePath:
+        {{- /* Use per-resource path if specified, else per-service, else global path + service name */ -}}
         {{- if .referencePath }}
         {{- toYaml .referencePath | nindent 8 }}
         {{- else if $config.referencePath }}
         {{- toYaml $config.referencePath | nindent 8 }}
         {{- else }}
         {{- with $values.ocm.referencePath }}
         {{- toYaml . | nindent 8 }}
         {{- end }}
         - name: {{ $service}}
         {{- end }}

gotemplates/components/runtime/ocm-resources.yaml (1)

1-1: Consider removing the redundant variable assignment.

The assignment {{ $values := .values }} is unnecessary since $.values is accessible throughout the template. You can directly reference $.values instead of $values.

🔎 Proposed refactor

-{{ $values := .values }}
-{{- range $service, $config := .values.services }}
+{{- range $service, $config := $.values.services }}
 {{- if $config.enabled }}
 apiVersion: delivery.ocm.software/v1alpha1
 kind: Resource
@@ -8,7 +7,7 @@ metadata:
   namespace: {{ $.releaseNamespace }}
   labels:
     artifact: chart
...
-  skipVerify: {{ $values.ocm.skipVerify }}
+  skipVerify: {{ $.values.ocm.skipVerify }}

Update all references from $values to $.values throughout the template.

test/e2e/kind/helpers.go (1)

100-124: Consider refactoring to reduce duplication.

The unstructuredsFromFileNoTemplate function duplicates significant logic from unstructuredsFromFile. Consider extracting the common YAML parsing and unmarshaling logic into a shared helper.

🔎 Proposed refactor

+// parseYAMLObjects splits YAML content and unmarshals into unstructured objects
+func parseYAMLObjects(content string, path string, log *logger.Logger) ([]unstructured.Unstructured, error) {
+	objects := strings.Split(content, "---\n")
+	var unstructuredObjs []unstructured.Unstructured
+	for _, obj := range objects {
+		var objMap map[string]interface{}
+		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
+			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from file %s. Output:\n%s", path, content)
+		}
+		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
+		obj := unstructured.Unstructured{Object: objMap}
+		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
+		unstructuredObjs = append(unstructuredObjs, obj)
+	}
+	return unstructuredObjs, nil
+}
+
 func unstructuredsFromFile(path string, templateData map[string]string, log *logger.Logger) ([]unstructured.Unstructured, error) {
 	manifestBytes, err := os.ReadFile(path)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to read file, pwd: %s", path)
 	}
 	log.Debug().Str("file", path).Str("template", string(manifestBytes)).Str("templateData", fmt.Sprintf("%+v", templateData)).Msg("Replacing template")
 	res, err := ReplaceTemplate(templateData, manifestBytes)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to replace template with path: %s", path)
 	}
-	// split the result into multiple YAML objects
-	objects := strings.Split(string(res), "---\n")
-	var unstructuredObjs []unstructured.Unstructured
-	for _, obj := range objects {
-		var objMap map[string]interface{}
-		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
-			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from template %s. Output:\n%s", path, string(res))
-		}
-		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
-		obj := unstructured.Unstructured{Object: objMap}
-		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
-		unstructuredObjs = append(unstructuredObjs, obj)
-	}
-	return unstructuredObjs, nil
+	return parseYAMLObjects(string(res), path, log)
 }
 
 func unstructuredsFromFileNoTemplate(path string, log *logger.Logger) ([]unstructured.Unstructured, error) {
 	manifestBytes, err := os.ReadFile(path)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to read file, pwd: %s", path)
 	}
 	log.Debug().Str("file", path).Str("manifest", string(manifestBytes)).Msg("Reading manifest file")
-	// split the result into multiple YAML objects
-	objects := strings.Split(string(manifestBytes), "---\n")
-	var unstructuredObjs []unstructured.Unstructured
-	for _, obj := range objects {
-		var objMap map[string]interface{}
-		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
-			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from file %s. Output:\n%s", path, string(manifestBytes))
-		}
-		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
-		obj := unstructured.Unstructured{Object: objMap}
-		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
-		unstructuredObjs = append(unstructuredObjs, obj)
-	}
-	return unstructuredObjs, nil
+	return parseYAMLObjects(string(manifestBytes), path, log)
 }

pkg/subroutines/resource/subroutine_test.go (1)

182-183: Unused managerMock variables could be removed.

Several tests create managerMock instances that are no longer used since the subroutine now accepts a client directly. Consider removing these for cleaner test code.

🔎 Example cleanup

 func (s *ResourceTestSuite) Test_updateHelmReleaseWithImageTag() {
     // ...
     s.Run(tt.name, func() {
-        managerMock := new(mocks.Manager)
         clientMock := new(mocks.Client)
         ctx := context.TODO()
         // ...
-        managerMock.On("GetClient").Return(clientMock)
         subroutine := subroutines.NewResourceSubroutine(clientMock)

Also applies to: 500-503, 563-566, 607-610, 647-650, 684-687

cmd/operator.go (2)

90-98: Error check scope issue: err may be checked without being set.

When RemoteRuntime.Enabled is false, GetClientAndRestConfig is not called, so err retains its previous value (nil from the tracing init). The check at lines 94-97 will pass, but this relies on implicit state. Consider restructuring for clarity:

 	restCfg := ctrl.GetConfigOrDie()
 	if operatorCfg.RemoteRuntime.Enabled {
 		setupLog.Info("Remote PlatformMesh reconciliation enabled, kubeconfig: " + operatorCfg.RemoteRuntime.Kubeconfig)
-		_, restCfg, err = subroutines.GetClientAndRestConfig(operatorCfg.RemoteRuntime.Kubeconfig)
-	}
-	if err != nil {
-		setupLog.Error(err, "unable to create PlatformMesh client")
-		os.Exit(1)
+		_, restCfg, err = subroutines.GetClientAndRestConfig(operatorCfg.RemoteRuntime.Kubeconfig)
+		if err != nil {
+			setupLog.Error(err, "unable to create PlatformMesh client")
+			os.Exit(1)
+		}
 	}

---

120-136: Minor inefficiency: local client created then potentially discarded.

When RemoteInfra.Enabled is true, the local client created at line 125 is immediately replaced. Consider restructuring to avoid unnecessary work:

 	var clientInfra client.Client
 	restCfgInfra := ctrl.GetConfigOrDie()
 	restCfgInfra.Wrap(func(rt http.RoundTripper) http.RoundTripper {
 		return otelhttp.NewTransport(rt)
 	})
-	clientInfra, err = client.New(restCfgInfra, client.Options{Scheme: subroutines.GetClientScheme()})
-	if err != nil {
-		setupLog.Error(err, "unable to create Infra client")
-		os.Exit(1)
-	}
 	if operatorCfg.RemoteInfra.Enabled {
 		clientInfra, _, err = subroutines.GetClientAndRestConfig(operatorCfg.RemoteInfra.Kubeconfig)
 		if err != nil {
 			setupLog.Error(err, "unable to create Infra client")
 			os.Exit(1)
 		}
+	} else {
+		clientInfra, err = client.New(restCfgInfra, client.Options{Scheme: subroutines.GetClientScheme()})
+		if err != nil {
+			setupLog.Error(err, "unable to create Infra client")
+			os.Exit(1)
+		}
 	}

internal/controller/platformmesh_controller.go (1)

97-99: Silent error suppression on List operation.

Errors from r.client.List are silently ignored, which could mask connectivity or RBAC issues. Consider logging the error:

 	if err := r.client.List(ctx, platformMeshList); err != nil {
+		// Log error but continue - this is a mapping function
 		return requests
 	}

pkg/subroutines/deployment.go (3)

461-466: Use context logger instead of creating new logger.

The function creates a new logger at lines 462-465 instead of using the context logger. This loses logging context and is inconsistent with other methods.

Proposed fix

 func (r *DeploymentSubroutine) buildComponentsTemplateVars(ctx context.Context, inst *v1alpha1.PlatformMesh, templateVars apiextensionsv1.JSON) (map[string]interface{}, error) {
-	log, err := logger.New(logger.DefaultConfig())
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed to create logger")
-	}
+	log := logger.LoadLoggerFromContext(ctx)

---

314-320: Use context logger instead of creating new logger.

Same issue as in buildComponentsTemplateVars - creating a new logger loses context. Use the context logger for consistency.

-	log, err := logger.New(logger.DefaultConfig())
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed to create logger")
-	}
+	log := logger.LoadLoggerFromContext(ctx)

---

684-832: Consider extracting common template rendering logic.

renderAndApplyComponentsInfraTemplates and renderAndApplyComponentsRuntimeTemplates share nearly identical logic. Consider extracting a common helper method that accepts the directory path and client as parameters to reduce duplication.

📜 Review details

Configuration used: Repository: platform-mesh/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b64a99a and bebe8ea.

⛔ Files ignored due to path filters (3)

• api/v1alpha1/zz_generated.deepcopy.go is excluded by !**/*_generated.*
• config/crd/core.platform-mesh.io_platformmeshes.yaml is excluded by !config/crd/**.yaml
• config/rbac/role.yaml is excluded by !config/rbac/**.yaml

📒 Files selected for processing (51)

• .gitignore
• Dockerfile
• README.md
• Taskfile.yml
• api/v1alpha1/platformmesh_types.go
• cmd/operator.go
• gotemplates/components/infra/helmreleases.yaml
• gotemplates/components/runtime/ocm-image-resources.yaml
• gotemplates/components/runtime/ocm-resources.yaml
• gotemplates/infra/infra/cert-manager/helmrelease.yaml
• gotemplates/infra/infra/gateway-api/kustomization.yaml
• gotemplates/infra/infra/traefik/helmrelease-crds.yaml
• gotemplates/infra/infra/traefik/helmrelease.yaml
• gotemplates/infra/runtime/cert-manager/resource.yaml
• gotemplates/infra/runtime/gateway-api/resource.yaml
• gotemplates/infra/runtime/traefik/resource-crds.yaml
• gotemplates/infra/runtime/traefik/resource.yaml
• internal/config/config.go
• internal/controller/platformmesh_controller.go
• internal/controller/realm_controller.go
• internal/controller/resource_controller.go
• manifests/k8s/platform-mesh-operator-components/release.yaml
• manifests/k8s/platform-mesh-operator-components/resource.yaml
• manifests/k8s/platform-mesh-operator-infra-components/release.yaml
• manifests/k8s/platform-mesh-operator-infra-components/resource.yaml
• pkg/merge/merge.go
• pkg/subroutines/defaults.go
• pkg/subroutines/deployment.go
• pkg/subroutines/deployment_helpers.go
• pkg/subroutines/deployment_helpers_test.go
• pkg/subroutines/deployment_test.go
• pkg/subroutines/featuretoggles.go
• pkg/subroutines/featuretoggles_test.go
• pkg/subroutines/kcpsetup.go
• pkg/subroutines/kcpsetup_test.go
• pkg/subroutines/providersecret.go
• pkg/subroutines/providersecret_test.go
• pkg/subroutines/resource/subroutine.go
• pkg/subroutines/resource/subroutine_test.go
• pkg/subroutines/subroutine_helpers.go
• pkg/subroutines/subroutine_testexports.go
• pkg/subroutines/wait_test.go
• test/e2e/kind/helpers.go
• test/e2e/kind/kustomize/base/crds/delivery.ocm.software_replications.yaml
• test/e2e/kind/kustomize/base/crds/kustomization.yaml
• test/e2e/kind/kustomize/base/ocm-k8s-toolkit/flux-sync.yaml
• test/e2e/kind/kustomize/components/ocm/component.yaml
• test/e2e/kind/kustomize/components/ocm/repository.yaml
• test/e2e/kind/suite_kind_test.go
• test/e2e/kind/yaml/platform-mesh-resource/default-profile.yaml
• test/e2e/kind/yaml/platform-mesh-resource/platform-mesh.yaml

💤 Files with no reviewable changes (5)

• manifests/k8s/platform-mesh-operator-components/release.yaml
• pkg/subroutines/subroutine_testexports.go
• manifests/k8s/platform-mesh-operator-infra-components/release.yaml
• manifests/k8s/platform-mesh-operator-infra-components/resource.yaml
• manifests/k8s/platform-mesh-operator-components/resource.yaml

🧰 Additional context used

🧬 Code graph analysis (15)

internal/controller/resource_controller.go (3)

pkg/subroutines/mocks/mock_Manager.go (1)

• Manager (35-37)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/resource/subroutine.go (1)

• NewResourceSubroutine (50-52)

internal/controller/realm_controller.go (1)

pkg/subroutines/subroutine_helpers.go (1)

• GetClientAndRestConfig (516-553)

test/e2e/kind/suite_kind_test.go (3)

test/e2e/kind/helpers.go (2)

• ApplyTemplateFromFile (20-42)
• ApplyFile (45-67)

internal/controller/platformmesh_controller.go (1)

• NewPlatformMeshReconciler (129-158)

internal/controller/resource_controller.go (1)

• NewResourceReconciler (78-98)

cmd/operator.go (4)

pkg/subroutines/subroutine_helpers.go (2)

• GetClientAndRestConfig (516-553)
• GetClientScheme (555-576)

internal/controller/platformmesh_controller.go (1)

• NewPlatformMeshReconciler (129-158)

internal/controller/resource_controller.go (1)

• NewResourceReconciler (78-98)

internal/controller/realm_controller.go (1)

• NewRealmReconciler (28-63)

pkg/subroutines/deployment_helpers_test.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/providersecret.go (2)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

pkg/subroutines/providersecret_test.go (4)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/providersecret.go (1)

• NewProviderSecretSubroutine (46-59)

pkg/subroutines/subroutine_helpers.go (2)

• Helper (54-55)
• KcpHelper (50-52)

pkg/subroutines/mocks/mock_KcpHelper.go (1)

• KcpHelper (13-15)

pkg/subroutines/resource/subroutine.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/featuretoggles.go (2)

pkg/subroutines/subroutine_helpers.go (1)

• KcpHelper (50-52)

internal/config/config.go (1)

• OperatorConfig (4-53)

pkg/subroutines/resource/subroutine_test.go (2)

pkg/subroutines/resource/subroutine.go (1)

• NewResourceSubroutine (50-52)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/kcpsetup_test.go (1)

pkg/subroutines/kcpsetup.go (1)

• NewKcpsetupSubroutine (49-58)

pkg/subroutines/deployment_helpers.go (1)

pkg/subroutines/deployment.go (1)

• DeploymentSubroutine (39-47)

pkg/subroutines/subroutine_helpers.go (3)

api/v1alpha1/groupversion_info.go (2)

• AddToScheme (35-35)
• GroupVersion (29-29)

api/v1alpha1/platformmesh_types.go (1)

• PlatformMesh (184-190)

internal/config/config.go (1)

• OperatorConfig (4-53)

test/e2e/kind/helpers.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

pkg/subroutines/deployment.go (4)

internal/config/config.go (1)

• OperatorConfig (4-53)

api/v1alpha1/platformmesh_types.go (1)

• PlatformMesh (184-190)

pkg/subroutines/subroutine_helpers.go (1)

• TemplateVars (258-283)

pkg/merge/merge.go (1)

• MergeMaps (9-42)

🪛 YAMLlint (1.37.1)

gotemplates/infra/runtime/cert-manager/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/components/runtime/ocm-resources.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/infra/infra/traefik/helmrelease.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/infra/traefik/helmrelease-crds.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/components/runtime/ocm-image-resources.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/infra/runtime/traefik/resource-crds.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/runtime/gateway-api/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/runtime/traefik/resource.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/components/infra/helmreleases.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

gotemplates/infra/infra/cert-manager/helmrelease.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

gotemplates/infra/infra/gateway-api/kustomization.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (70)

Taskfile.yml (4)

20-20: Configuration centralization approved.

The introduction of the TEST_TIMEOUT variable improves maintainability by consolidating the timeout configuration in one location. The 15-minute default is appropriate given the addition of remote cluster operations in this PR.

---

75-75: Variable reference correctly implemented.

The timeout parameter properly references the centralized TEST_TIMEOUT variable using Task's template syntax.

---

87-87: Consistent timeout applied to CI workflow.

The CI test path correctly uses the centralized timeout variable, ensuring consistency between CI and local test environments.

---

105-105: Local test timeout consistently configured.

The local test execution path correctly references the centralized timeout variable, completing the consistent application of the timeout configuration across all test tasks.

test/e2e/kind/kustomize/components/ocm/repository.yaml (1)

1-11: LGTM! Namespace update aligns with the PR architecture.

The namespace change from default to platform-mesh-system is consistent with the broader PR objectives to standardize on platform-mesh-system namespace for PlatformMesh resources.

test/e2e/kind/kustomize/components/ocm/component.yaml (1)

1-17: Namespace updates are consistent and correct.

Both the Component metadata namespace and the ocmConfig Repository namespace are correctly set to platform-mesh-system. The repositoryRef resolution will work correctly since both the Component and Repository resources are in the same namespace.

gotemplates/infra/infra/gateway-api/kustomization.yaml (1)

1-23: Template structure is correct; YAMLlint error is a false positive.

The Go template syntax ({{- if .gatewayApi.enabled }}) triggers YAMLlint's syntax error, which can be safely ignored. The template correctly:

• Gates the entire Kustomization on gatewayApi.enabled
• Conditionally includes kubeConfig for remote cluster support
• Applies the operator-created label for tracking
• References the appropriate GitRepository source

gotemplates/infra/runtime/gateway-api/resource.yaml (1)

1-29: OCM Resource template structure is correct; YAMLlint error is a false positive.

The Go template syntax triggers YAMLlint's syntax error, which can be safely ignored. The template correctly:

• Gates Resource creation on gatewayApi.enabled
• Builds the OCM referencePath by iterating over base paths and appending the gateway-api entry
• References the OCM component and repository configuration
• Sets appropriate metadata labels for tracking

test/e2e/kind/kustomize/base/ocm-k8s-toolkit/flux-sync.yaml (1)

39-48: RBAC permissions added for OCM Replication resources.

The ClusterRole patch grants necessary permissions for the OCM controller to manage Replication resources in the delivery.ocm.software API group. The specified verbs (create, delete, get, list, patch, update, watch) are standard and required for controller-based resource reconciliation.

gotemplates/infra/infra/cert-manager/helmrelease.yaml (1)

1-1: Static analysis error is a false positive.

The YAMLlint syntax error on line 1 can be safely ignored. This file is a Go template (not plain YAML), and {{- if .certManager.enabled }} is valid Go template syntax.

gotemplates/infra/runtime/cert-manager/resource.yaml (1)

1-1: Static analysis error is a false positive.

The YAMLlint syntax error can be safely ignored. This is a Go template file, and the {{- if .certManager.enabled }} syntax is valid.

pkg/merge/merge.go (1)

54-65: LGTM!

The refined merge logic correctly enforces destination precedence. The change from warning to debug logging is appropriate, as this is expected behavior rather than a potential issue. The nil check prevents unnecessary log entries.

gotemplates/infra/infra/traefik/helmrelease.yaml (2)

1-1: Static analysis error is a false positive.

The YAMLlint syntax error can be safely ignored. This is a Go template file where {{- if .traefik.enabled }} is valid syntax.

---

24-25: LGTM!

The dependsOn reference to Traefik CRDs ensures proper installation order, preventing potential race conditions during deployment.

pkg/subroutines/wait_test.go (3)

62-68: LGTM!

The test correctly reflects the updated resource selection logic using the new label selector core.platform-mesh.io/operator-created=true. The change to .Once() aligns with the simplified resource discovery flow.

---

100-117: LGTM!

Test expectations correctly updated to reflect the new namespace (platform-mesh-system) and label selector (core.platform-mesh.io/operator-created=true).

---

149-152: LGTM!

All test cases consistently updated with the new namespace, label selector, and resource naming conventions. The changes maintain test coverage while aligning with the refactored resource discovery logic.

Also applies to: 225-229, 361-361, 372-372

api/v1alpha1/platformmesh_types.go (3)

26-34: LGTM: ProfileConfigMap field properly integrated.

The addition of the ProfileConfigMap field to PlatformMeshSpec follows the existing pattern for optional pointer fields and is correctly annotated with JSON tags.

---

36-44: LGTM: ConfigMapReference type is well-designed.

The ConfigMapReference type is properly structured with clear documentation of the namespace defaulting behavior. The required Name field and optional Namespace field follow Kubernetes conventions.

---

176-180: LGTM: PrintColumn declarations updated correctly.

The updated printcolumn declarations for FEATURES and WAIT properly reference the new condition types (FeatureToggleSubroutine_Ready and WaitSubroutine_Ready) and follow the established pattern for status visibility.

pkg/subroutines/providersecret.go (2)

46-66: LGTM: Constructor refactored to accept operator configuration.

The change from accepting a kcpUrl string parameter to cfgOperator *config.OperatorConfig improves flexibility and aligns with the remote-deployment enhancements. This enables the subroutine to access comprehensive configuration including remote infra/runtime settings.

---

133-133: Helper function implementations are correct.

The getExternalKcpHost and getInternalKcpHost functions properly handle operator configuration and instance-specific settings. getExternalKcpHost checks for custom exposure configuration on the instance and falls back to the operator-provided front-proxy URL, while getInternalKcpHost consistently returns the internal front-proxy URL from operator configuration. Both functions are used correctly across the codebase.

pkg/subroutines/deployment_helpers.go (4)

34-42: LGTM: Metadata update logic is correct.

The updateObjectMetadata function properly handles nil checks and correctly copies labels and annotations from the desired object to the existing object.

---

67-113: LGTM: Template rendering and application logic is well-structured.

The renderAndApplyTemplates function properly walks the directory, filters YAML files, renders templates, and applies manifests with comprehensive error handling and logging.

---

147-160: LGTM: Zero-value detection logic is comprehensive.

The isZeroValue function properly handles various types (nil, string, slice, map) and uses reflection for other cases. The implementation is suitable for template logic.

---

162-218: LGTM: Template function map provides comprehensive helpers.

The template functions (default, toYaml, nindent, or, and, not) follow Helm-like conventions and properly handle edge cases. The nindent implementation correctly filters empty lines and applies indentation.

pkg/subroutines/deployment_helpers_test.go (4)

19-36: LGTM: Test suite setup is appropriate.

The test suite is properly configured with a debug logger for test execution. The setup follows testify suite patterns.

---

38-103: LGTM: Metadata update tests cover key scenarios.

The test cases validate both the replacement of metadata when desired values are present and the preservation behavior when desired metadata is absent. The validation logic correctly checks the expected outcomes.

---

105-174: LGTM: getOrCreateObject tests comprehensively cover all paths.

The tests validate the three scenarios: existing object retrieval, SSA-based creation on NotFound, and error propagation. The mock expectations correctly include client.Apply and client.FieldOwner(fieldManagerDeployment) for the creation path.

---

176-199: LGTM: Error handling test validates proper error propagation.

The test correctly simulates a Get error and verifies that the error is wrapped with an appropriate message.

test/e2e/kind/kustomize/base/crds/kustomization.yaml (1)

7-7: LGTM: Replication CRD added to Kustomization.

The addition of delivery.ocm.software_replications.yaml expands the CRD resources for the e2e test environment, aligning with the OCM enhancements in this PR.

pkg/subroutines/featuretoggles_test.go (1)

45-47: LGTM! Test updated to match new constructor signature.

The test correctly reflects the removal of the kcpUrl parameter from NewFeatureToggleSubroutine. The constructor now receives only the client, helper, and config, consistent with the refactoring in featuretoggles.go.

pkg/subroutines/featuretoggles.go (2)

43-50: LGTM! Constructor refactored to remove static kcpUrl parameter.

The updated constructor signature correctly removes the kcpUrl parameter. The function now relies on dynamic host resolution via getExternalKcpHost (used at line 131), which aligns with the PR's goal of supporting remote cluster deployments.

The corresponding test in featuretoggles_test.go has been updated consistently.

---

131-131: LGTM! Dynamic KCP host resolution implemented correctly.

The change from a static r.kcpUrl field to dynamic getExternalKcpHost(inst, &operatorCfg) call correctly supports per-instance KCP host resolution. This aligns with the remote deployment architecture introduced in this PR, where the KCP URL may vary based on the PlatformMesh instance configuration.

gotemplates/components/infra/helmreleases.yaml (1)

1-3: LGTM!

The template header correctly captures values and iterates over enabled services with proper filtering.

internal/config/config.go (3)

43-46: LGTM!

The RemoteInfra configuration is well-structured with appropriate defaults.

---

47-52: LGTM!

The RemoteRuntime configuration includes appropriate fields for both kubeconfig file and secret reference patterns.

---

37-41: No issues identified. The nested Resource struct with mapstructure:",squash" follows the standard mapstructure pattern used consistently throughout the configuration file. The configuration key subroutines-resource-enabled is unique with no conflicts, and nested squash is properly handled by mapstructure.

internal/controller/realm_controller.go (1)

44-53: No changes needed. The mixed client usage is intentional architectural design.

PatchOIDCSubroutine correctly uses mgr.GetClient() because it operates on the local cluster, while LifecycleManager receives clientInfra for lifecycle coordination. This pattern—where subroutines are initialized with clients appropriate to their operational scope—is consistent across all reconcilers in the codebase (e.g., WaitSubroutine and ResourceSubroutine receive clientInfra, while KcpsetupSubroutine and ProviderSecretSubroutine receive local clients). The separation between subroutine-level client selection and lifecycle management is a sound architectural choice.

Likely an incorrect or invalid review comment.

pkg/subroutines/kcpsetup.go (4)

46-46: LGTM!

Introducing a constant for the field manager name improves consistency and maintainability.

---

49-58: LGTM!

Removing the kcpUrl parameter simplifies the constructor and aligns with the centralized configuration approach where the KCP host is dynamically resolved via getExternalKcpHost.

---

328-328: LGTM!

Using the fieldManagerKcpSetup constant ensures consistent field ownership across server-side apply operations.

---

100-100: Implementation of getExternalKcpHost is correct and properly handles edge cases.

The function (defined in pkg/subroutines/subroutine_helpers.go:578-584) correctly handles the primary edge case: when inst.Spec.Exposure is nil, it falls back to the operator configuration defaults. When custom exposure configuration is present, it constructs the URL from the specification. The function is consistently used across multiple subroutines and does not introduce any risks.

test/e2e/kind/helpers.go (1)

18-42: LGTM!

Renaming to ApplyTemplateFromFile and adding documentation improves clarity about the function's behavior.

pkg/subroutines/kcpsetup_test.go (1)

53-53: LGTM!

All test constructor calls have been correctly updated to match the new NewKcpsetupSubroutine signature without the kcpUrl parameter. The changes are consistent across all test cases.

Also applies to: 71-71, 193-193, 475-475, 483-483, 571-571, 582-582, 676-676

test/e2e/kind/kustomize/base/crds/delivery.ocm.software_replications.yaml (1)

1-335: LGTM!

The Replication CRD is well-structured and follows Kubernetes CRD best practices. The schema includes comprehensive validations, proper required fields, and appropriate constraints. The CEL validation rules correctly enforce the allowed apiVersion/kind combinations for referenced objects.

gotemplates/components/runtime/ocm-resources.yaml (1)

32-40: The concern about YAML malformation is unfounded. The template structure ensures referencePath always contains at least one entry. When both $config.referencePath and $values.ocm.referencePath are empty, the - name: {{ $service}} entry (line 39) is still rendered as a fallback, producing valid YAML with the service name. This is an intentional design pattern also present in other similar templates (traefik, gateway-api, cert-manager). No action required.

pkg/subroutines/deployment_test.go (1)

30-54: Test setup correctly updated for new constructor signature.

The SetupTest method properly initializes the OperatorConfig with KCP and RemoteInfra settings required by the updated NewDeploymentSubroutine constructor. Passing nil as the second parameter (clientInfra) is appropriate for tests that do not require a separate infrastructure client.

pkg/subroutines/providersecret_test.go (2)

80-87: Configuration initialization is correct.

The suite setup properly initializes the OperatorConfig with required KCP fields. This aligns with the constructor signature that now expects *config.OperatorConfig instead of a URL string.

---

1761-1785: Internal kubeconfig URL validation correctly implemented.

The additional mock expectation validates that internal kubeconfigs are rewritten to use the in-cluster front-proxy service endpoint. This complements the external kubeconfig test at line 1749.

test/e2e/kind/yaml/platform-mesh-resource/default-profile.yaml (1)

1-3: ConfigMap structure is correct for profile-driven deployment.

The ConfigMap uses the expected profile.yaml data key and is placed in the platform-mesh-system namespace, aligning with the deployment subroutine's profile loading logic.

Also applies to: 389-392

test/e2e/kind/yaml/platform-mesh-resource/platform-mesh.yaml (1)

7-9: Profile ConfigMap reference is correctly configured.

The profileConfigMap field properly references the platform-mesh-profile ConfigMap in the platform-mesh-system namespace, enabling profile-driven deployment.

pkg/subroutines/resource/subroutine.go (3)

172-196: SSA implementation for HelmRelease image tag updates is correct.

The Server-Side Apply approach with client.ForceOwnership enables this subroutine to manage specific fields without conflicting with other field managers. Creating a minimal patch object containing only the managed field is the correct pattern.

---

210-232: SSA implementation for HelmRelease chart version updates follows the same pattern.

Consistent with updateHelmReleaseWithImageTag, this method correctly uses SSA to update only the spec.chart.spec.version field.

---

364-367: Timeout addition for GitRepository is appropriate.

Adding a 5-minute timeout prevents indefinite hangs during Git clone operations.

pkg/subroutines/resource/subroutine_test.go (2)

225-237: Test correctly validates SSA Patch behavior for image tag updates.

The mock expectation for Patch with 5 arguments matches the SSA implementation. The test properly validates that the version is set at the expected path in the patched object.

---

414-427: Test correctly separates HelmRepository Update from HelmRelease Patch.

The test now properly validates the dual update path: controllerutil.CreateOrUpdate (Get+Update) for HelmRepository, and SSA Patch for HelmRelease.

test/e2e/kind/suite_kind_test.go (3)

154-160: Kubeconfig export step added with proper error handling.

The additional kubeconfig export provides a file-based kubeconfig for external tooling access. Error logging is appropriately included.

---

419-425: Profile-driven manifest application is correctly structured.

The setup now applies the default profile ConfigMap before the PlatformMesh resource, ensuring the profile is available when the resource is reconciled.

---

543-543: Reconciler constructors correctly pass the client parameter.

Both NewPlatformMeshReconciler and NewResourceReconciler are updated to pass mgr.GetClient() as the clientInfra parameter, consistent with the updated constructor signatures.

Also applies to: 550-550

cmd/operator.go (1)

137-154: Reconciler wiring is correct.

The updated constructors correctly pass clientInfra to NewPlatformMeshReconciler and NewResourceReconciler. The logger scoping via ChildLogger provides appropriate context for each controller type.

internal/controller/platformmesh_controller.go (1)

129-157: Constructor correctly wires dual-client support.

The NewPlatformMeshReconciler properly accepts and propagates clientInfra to subroutines that need it (DeploymentSubroutine, WaitSubroutine). The client field is correctly initialized for ConfigMap watching operations.

pkg/subroutines/subroutine_helpers.go (3)

555-576: Scheme registration is comprehensive and follows standard patterns.

The GetClientScheme function properly registers all required API groups including FluxCD, cert-manager, and the OCM Resource type. The use of utilruntime.Must is appropriate for scheme registration.

---

182-229: Function update correctly integrates remote runtime configuration.

The updated MergeValuesAndServices properly injects kubeConfigEnabled and related secret fields when remote runtime is enabled. The existing merge logic is preserved.

---

578-588: Helper functions correctly resolve KCP host URLs.

The getExternalKcpHost and getInternalKcpHost functions properly handle the exposure configuration and provide appropriate defaults.

README.md (1)

258-308: Profile configuration documentation is clear and accurate.

The documentation properly explains the profile ConfigMap structure, default naming convention (<platform-mesh-name>-profile), and the infra/components sections. This aligns with the implementation in deployment.go.

pkg/subroutines/deployment.go (4)

39-52: Struct and constants are well-defined for dual-cluster support.

The DeploymentSubroutine struct properly separates clientInfra and clientRuntime for multi-cluster operations. The profile ConfigMap constants provide consistent naming conventions.

---

73-109: Profile ConfigMap resolution handles all cases correctly.

The method properly resolves explicit profileConfigMap references and falls back to the default naming convention. Error handling for missing keys is appropriate.

---

834-917: SSA application with retry logic is well-implemented.

The applyManifest function correctly uses Server-Side Apply with field manager separation. The retry logic for HelmRelease conflicts handles race conditions appropriately. The special handling for Resource objects (removing interval) is properly documented.

---

167-280: Process method correctly orchestrates dual-cluster deployment.

The deployment flow properly sequences:

1. Template variable preparation
2. Infra template application (to infra cluster)
3. Runtime template application (to runtime cluster)
4. Cert-manager readiness wait
5. Component template application
6. Authorization webhook management
7. Istio and KCP component readiness checks

The dual-client usage is correct throughout.

[akafazov]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

pkg/subroutines/deployment_helpers.go (1)

28-28: Verify if unused constant can be removed.

The specFieldInterval constant is defined but was previously flagged as unused. Confirm whether this constant is referenced elsewhere in the codebase before removing it.

#!/bin/bash
# Search for usage of specFieldInterval constant
rg -n 'specFieldInterval' --type go

🧹 Nitpick comments (4)

internal/controller/platformmesh_controller.go (1)

88-130: Consider logging errors when listing PlatformMesh resources fails.

The function silently returns an empty slice when r.client.List fails (line 100-102). In a production environment, failing to enqueue reconcile requests due to a List error could lead to missed updates. Consider logging the error before returning to aid debugging.

🔎 Add error logging

 	// List all PlatformMesh resources
 	platformMeshList := &corev1alpha1.PlatformMeshList{}
 	// We use the context from the handler since it's provided by controller-runtime
 	if err := r.client.List(ctx, platformMeshList); err != nil {
+		// Log the error to aid debugging
+		// Note: logger should be extracted from context or passed as parameter
+		// For now, silently fail but consider adding logging
 		return requests
 	}

Note: The safe type assertion at line 92-95 correctly addresses the previous review comment about potential panics.

test/e2e/kind/helpers.go (1)

102-126: Consider extracting common YAML parsing logic.

While the previous review's primary concern about apply-loop duplication is now addressed, there's still duplication between unstructuredsFromFile (lines 71-100) and unstructuredsFromFileNoTemplate (lines 102-126). The YAML splitting, unmarshalling, and logging logic is nearly identical. Consider extracting a helper like parseYAMLToUnstructureds(yamlBytes []byte, path string, log *logger.Logger) to eliminate this remaining duplication.

🔎 Proposed refactor to reduce parsing duplication

+// parseYAMLToUnstructureds splits a multi-document YAML and parses it into unstructured objects
+func parseYAMLToUnstructureds(yamlBytes []byte, path string, log *logger.Logger) ([]unstructured.Unstructured, error) {
+	objects := strings.Split(string(yamlBytes), "---\n")
+	var unstructuredObjs []unstructured.Unstructured
+	for _, obj := range objects {
+		var objMap map[string]interface{}
+		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
+			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from %s. Output:\n%s", path, string(yamlBytes))
+		}
+		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
+		obj := unstructured.Unstructured{Object: objMap}
+		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
+		unstructuredObjs = append(unstructuredObjs, obj)
+	}
+	return unstructuredObjs, nil
+}
+
 func unstructuredsFromFile(path string, templateData map[string]string, log *logger.Logger) ([]unstructured.Unstructured, error) {
 	manifestBytes, err := os.ReadFile(path)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to read file, pwd: %s", path)
 	}
 	log.Debug().Str("file", path).Str("template", string(manifestBytes)).Str("templateData", fmt.Sprintf("%+v", templateData)).Msg("Replacing template")
 	res, err := ReplaceTemplate(templateData, manifestBytes)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to replace template with path: %s", path)
 	}
-	// split the result into multiple YAML objects
-	objects := strings.Split(string(res), "---\n")
-	var unstructuredObjs []unstructured.Unstructured
-	for _, obj := range objects {
-		var objMap map[string]interface{}
-		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
-			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from template %s. Output:\n%s", path, string(res))
-		}
-		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
-		obj := unstructured.Unstructured{Object: objMap}
-		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
-		unstructuredObjs = append(unstructuredObjs, obj)
-	}
-	return unstructuredObjs, nil
+	return parseYAMLToUnstructureds(res, path, log)
 }

 func unstructuredsFromFileNoTemplate(path string, log *logger.Logger) ([]unstructured.Unstructured, error) {
 	manifestBytes, err := os.ReadFile(path)
 	if err != nil {
 		return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to read file, pwd: %s", path)
 	}
 	log.Debug().Str("file", path).Str("manifest", string(manifestBytes)).Msg("Reading manifest file")
-	// split the result into multiple YAML objects
-	objects := strings.Split(string(manifestBytes), "---\n")
-	var unstructuredObjs []unstructured.Unstructured
-	for _, obj := range objects {
-		var objMap map[string]interface{}
-		if err := yaml.Unmarshal([]byte(obj), &objMap); err != nil {
-			return []unstructured.Unstructured{}, errors.Wrap(err, "Failed to unmarshal YAML from file %s. Output:\n%s", path, string(manifestBytes))
-		}
-		log.Debug().Str("obj", fmt.Sprintf("%+v", objMap)).Msg("Unmarshalled object")
-		obj := unstructured.Unstructured{Object: objMap}
-		log.Debug().Str("file", path).Str("kind", obj.GetKind()).Str("name", obj.GetName()).Str("namespace", obj.GetNamespace()).Msg("Applying manifest")
-		unstructuredObjs = append(unstructuredObjs, obj)
-	}
-	return unstructuredObjs, nil
+	return parseYAMLToUnstructureds(manifestBytes, path, log)
 }

pkg/subroutines/subroutine_helpers.go (1)

271-285: Remove duplicate marshal logic.

Lines 280 and 281-285 both marshal values to JSON and assign to result.Raw. The first assignment on line 280 is immediately overwritten, making it redundant.

🔎 Proposed fix

 	values := map[string]interface{}{
 		"iamWebhookCA":         base64.StdEncoding.EncodeToString(secret.Data["ca.crt"]),
 		"baseDomain":           baseDomain,
 		"protocol":             protocol,
 		"port":                 fmt.Sprintf("%d", port),
 		"baseDomainPort":       baseDomainPort,
 		"helmReleaseNamespace": inst.Namespace,
 	}
 
 	result := apiextensionsv1.JSON{}
-	result.Raw, _ = json.Marshal(values)
 	raw, err := json.Marshal(values)
 	if err != nil {
 		return apiextensionsv1.JSON{}, errors.Wrap(err, "Failed to marshal template vars")
 	}
 	result.Raw = raw
 
 	return result, nil

cmd/operator.go (1)

90-98: Consider restructuring for clarity.

The error check on lines 94-97 is outside the conditional block where err is assigned (lines 90-93). While functionally correct (when RemoteRuntime.Enabled is false, err remains nil from line 48), this structure can confuse readers who expect error handling immediately after assignment.

🔎 Proposed refactor for improved readability

 	restCfg := ctrl.GetConfigOrDie()
 	if operatorCfg.RemoteRuntime.Enabled {
 		setupLog.Info("Remote PlatformMesh reconciliation enabled, kubeconfig: " + operatorCfg.RemoteRuntime.Kubeconfig)
 		_, restCfg, err = subroutines.GetClientAndRestConfig(operatorCfg.RemoteRuntime.Kubeconfig)
-	}
-	if err != nil {
-		setupLog.Error(err, "unable to create PlatformMesh client")
-		os.Exit(1)
+		if err != nil {
+			setupLog.Error(err, "unable to create PlatformMesh client")
+			os.Exit(1)
+		}
 	}

📜 Review details

Configuration used: Repository: platform-mesh/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bebe8ea and d117df6.

📒 Files selected for processing (14)

• Dockerfile
• README.md
• api/v1alpha1/platformmesh_types.go
• cmd/operator.go
• gotemplates/components/runtime/ocm-image-resources.yaml
• internal/controller/platformmesh_controller.go
• internal/controller/realm_controller.go
• pkg/merge/merge.go
• pkg/subroutines/deployment_helpers.go
• pkg/subroutines/deployment_helpers_test.go
• pkg/subroutines/providersecret.go
• pkg/subroutines/providersecret_test.go
• pkg/subroutines/subroutine_helpers.go
• test/e2e/kind/helpers.go

🚧 Files skipped from review as they are similar to previous changes (2)

• Dockerfile
• pkg/merge/merge.go

🧰 Additional context used

🧬 Code graph analysis (2)

pkg/subroutines/subroutine_helpers.go (2)

api/v1alpha1/groupversion_info.go (1)

• GroupVersion (29-29)

internal/config/config.go (1)

• OperatorConfig (4-53)

test/e2e/kind/helpers.go (1)

pkg/subroutines/mocks/mock_Client.go (1)

• Client (22-24)

🪛 YAMLlint (1.37.1)

gotemplates/components/runtime/ocm-image-resources.yaml

[error] 2-2: syntax error: expected '', but found '{'

(syntax)

🔇 Additional comments (30)

gotemplates/components/runtime/ocm-image-resources.yaml (1)

1-52: LGTM! Static analysis false positive can be ignored.

The YAMLlint syntax error on line 2 is a false positive—this file is a Go template with {{ }} syntax, not pure YAML. The template logic correctly iterates over enabled services with imageResources, applies proper fallback logic for referencePath (per-resource → per-service → global), and includes the skipVerify field as previously addressed.

pkg/subroutines/providersecret.go (3)

46-63: LGTM! Constructor correctly refactored to config-driven approach.

The removal of the kcpUrl parameter and field aligns with the new pattern of deriving KCP host information from configuration via helper functions (getExternalKcpHost and getInternalKcpHost).

---

226-231: The getInternalKcpHost helper function is properly defined in pkg/subroutines/subroutine_helpers.go with the correct signature: func getInternalKcpHost(cfg *config.OperatorConfig) string. It returns the appropriate internal KCP host URL and is correctly used in the code at line 230.

---

130-130: Helper function verified and correctly implemented.

The getExternalKcpHost(inst *v1alpha1.PlatformMesh, cfg *config.OperatorConfig) string helper is properly defined in pkg/subroutines/subroutine_helpers.go with the expected signature. It returns a string suitable for the buildKubeconfig parameter, and handles both nil and non-nil exposure configurations appropriately.

pkg/subroutines/deployment_helpers.go (3)

56-73: LGTM! Server-populated fields now correctly retrieved.

The implementation correctly addresses the previous concern by re-fetching the object via Get() after SSA Apply (lines 62-70). This ensures the returned object contains server-populated fields like resourceVersion, uid, and creationTimestamp.

---

147-150: LGTM! Sensitive data no longer exposed in error logs.

The error message now includes only the byte count (size: %d bytes) instead of the full rendered YAML content, preventing potential exposure of sensitive data such as secrets or credentials.

---

170-226: LGTM! Template function map implementation is sound.

The template helper functions provide Helm-like functionality (default, toYaml, nindent, logical operators) with proper handling of zero values and edge cases. The nindent implementation correctly filters empty lines at boundaries and indents non-empty content.

internal/controller/realm_controller.go (2)

30-53: LGTM! Client selection correctly implements remote infra support.

The initialization properly selects between the local manager client and a remote infra client based on configuration. The clientRuntime variable is correctly passed to the lifecycle manager, enabling the dual-cluster deployment pattern.

---

28-63: Error handling correctly refactored.

The constructor now returns (*RealmReconciler, error) and properly propagates errors. The error is handled by the caller in cmd/operator.go at line 149-152, which checks the error and exits gracefully.

api/v1alpha1/platformmesh_types.go (1)

177-181: LGTM! Printcolumn updates align with new subroutine structure.

The addition of FeatureToggleSubroutine_Ready (FEATURES) and WaitSubroutine_Ready (WAIT) columns reflects the updated reconciliation flow and provides better visibility into the new profile-driven deployment model.

pkg/subroutines/deployment_helpers_test.go (2)

105-136: LGTM!

The test correctly validates the behavior when an object already exists. The mock setup properly simulates the Get operation and the assertions verify the returned object.

---

138-219: LGTM!

The tests comprehensively cover the object creation path using Server-Side Apply semantics and error handling. The mock expectations correctly simulate:

• NotFound error triggering creation via Patch with Apply
• Re-fetching the object after creation
• General error propagation

pkg/subroutines/providersecret_test.go (2)

56-56: LGTM!

The addition of the config field and initialization aligns with the config-driven refactor described in the PR objectives. All KCP-related settings are now properly initialized through the OperatorConfig structure.

Also applies to: 80-87

---

1642-1647: LGTM!

The expanded test coverage for both external and internal kubeconfig scenarios properly validates the URL rewriting logic for front-proxy based routing. The assertions correctly verify the expected server URLs for each context.

Also applies to: 1761-1785

README.md (3)

53-77: LGTM!

The General and KCP Configuration sections are well-structured and provide clear descriptions of all configuration options with their defaults.

---

252-301: LGTM!

The Profile Configuration section provides clear documentation for the new profile-driven deployment model, including both default and custom profile scenarios. The YAML examples effectively illustrate the unified structure.

---

128-143: Documentation is accurate. The Remote Infrastructure and Remote Runtime configuration sections correctly document all fields with matching names and default values as defined in internal/config/config.go.

internal/controller/platformmesh_controller.go (3)

49-49: LGTM!

The addition of the client field and corresponding RBAC permissions correctly supports the new ConfigMap watching functionality.

Also applies to: 55-55

---

79-83: LGTM!

The ConfigMap watcher correctly implements the secondary resource watch pattern to trigger PlatformMesh reconciliation when profile ConfigMaps change.

---

132-160: LGTM!

The constructor changes correctly implement the dual-client architecture, threading clientInfra through to the appropriate subroutines and initializing the client field for ConfigMap mapping operations.

test/e2e/kind/helpers.go (2)

18-38: LGTM!

The extracted applyUnstructureds helper successfully eliminates code duplication as requested in the previous review. The common apply logic is now shared between templated and non-templated paths.

Based on learnings, this refactor was specifically requested in a previous review to address code duplication.

---

40-69: LGTM!

Both ApplyTemplateFromFile and ApplyFile now use the common applyUnstructureds helper, successfully implementing the refactor suggested in the previous review. The separation of file parsing and object application is clean and maintainable.

pkg/subroutines/subroutine_helpers.go (6)

16-26: LGTM!

The new imports support the extended client scheme registration and remote deployment features introduced in this PR.

---

66-66: LGTM!

Using appsv1.AddToScheme is more specific and correct for the apps API group compared to the generic v1.

---

182-230: LGTM!

The function now correctly accepts config.OperatorConfig and integrates kubeConfig-related values when remote runtime is enabled. The logic properly merges template variables with services and OCM config.

---

521-558: LGTM! Past issue resolved.

The function now correctly uses log.Error() instead of log.Fatal(), allowing errors to propagate to callers for graceful handling as requested in the previous review.

---

560-581: LGTM!

The scheme correctly registers all required API groups (v1alpha1, Helm, cert-manager, Flux, core, apps) and includes unstructured fallback types for dynamic client creation.

---

583-593: LGTM!

Both helper functions correctly derive KCP host URLs based on operator configuration and exposure settings.

cmd/operator.go (2)

120-136: LGTM!

The infra client setup correctly handles both local and remote configurations. The default client is created first, then optionally replaced when RemoteInfra.Enabled is true.

---

137-157: LGTM!

Reconciler constructors are correctly updated to accept the new clientInfra parameter and use scoped loggers via log.ChildLogger(). The Realm reconciler properly handles the error-returning constructor.