[Feature] Centralize DBaaS operator configuration with nil-go/konf - #245
Conversation
- add typed configuration hierarchy, defaults, and validation - support file, environment, and explicit flag precedence - add optional ConfigMap-backed Kustomize overlay - keep the default deployment independent of a ConfigMap - centralize database defaults and remove duplicated fallbacks - wire resolved configuration into controllers and dependencies - add configuration and precedence tests
…rvice configuration
… image configuration
📝 WalkthroughWalkthroughThe operator now uses typed, validated configuration loaded from defaults, JSON, environment variables, and explicit flags. Startup, Kubernetes manifests, reconciliation defaults, monitoring, gateway namespace handling, and Harvester storage validation are wired to the centralized configuration. ChangesOperator configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Pod
participant ConfigLoader
participant ControllerManager
participant DBInstanceReconciler
participant KubernetesAPI
Pod->>ConfigLoader: load defaults, file, environment, and flags
ConfigLoader-->>Pod: validated Config and PodNamespace
Pod->>ControllerManager: start configured manager and servers
ControllerManager->>DBInstanceReconciler: provide defaults and monitoring settings
DBInstanceReconciler->>KubernetesAPI: reconcile DBInstance resources
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Yohansenanayake
changed the title
nil-go/konfnil-go/konf
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
database/internal/ensure/defaults_test.go (1)
48-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a non-default configuration.
All updated cases pass built-in defaults, so they cannot detect
immutableDriftWithDefaultsignoring its supplied configuration. Add a case with custom database defaults and omitted spec fields that matchAppliedSpec.Also applies to: 74-76, 96-98, 122-124, 150-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/internal/ensure/defaults_test.go` around lines 48 - 50, Add test cases covering custom database defaults passed to immutableDriftWithDefaults, with omitted spec fields matching the corresponding AppliedSpec values and no reported drift. Update the related cases at the referenced assertions to use the custom configuration rather than only operatorconfig.Default().DatabaseDefaults, while preserving existing default-configuration coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@database/config/base/cert_metrics_manager_patch.yaml`:
- Around line 11-14: Replace the stale --metrics-cert-path argument in the
metrics server container args with the registered observability.metrics.tls.*
flag for the mounted certificate directory, preserving the existing
/tmp/k8s-metrics-server/metrics-certs path.
In `@database/config/overlays/operator-config/kustomization.yaml`:
- Around line 8-11: Update the dbaas-controller image entry in
kustomization.yaml to use an already-published immutable tag dated no later than
July 28, 2026, replacing dev-b658cf2-202607290025. Keep the existing image
repository unchanged.
In `@database/internal/config/validate.go`:
- Around line 37-39: Update the validateBindAddress call for
server.health.bindAddress in config validation to allow the "0" disable sentinel
by enabling the corresponding acceptance flag. Preserve the existing validation
and error propagation for all other bind-address values.
In `@database/internal/controller/controller_test_helpers_test.go`:
- Around line 49-54: Update resetEnsureRunner to pass the configured
DatabaseDefaults, InstanceClasses, and Monitoring dependencies from the runner
into ensure.Dependencies, matching production wiring and avoiding fallback
values in controller tests.
---
Nitpick comments:
In `@database/internal/ensure/defaults_test.go`:
- Around line 48-50: Add test cases covering custom database defaults passed to
immutableDriftWithDefaults, with omitted spec fields matching the corresponding
AppliedSpec values and no reported drift. Update the related cases at the
referenced assertions to use the custom configuration rather than only
operatorconfig.Default().DatabaseDefaults, while preserving existing
default-configuration coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cf9de88-99b6-4c68-abc6-66cb539b4bd3
⛔ Files ignored due to path filters (1)
database/go.sumis excluded by!**/*.sum
📒 Files selected for processing (42)
database/README.mddatabase/cmd/main.godatabase/config/base/cert_metrics_manager_patch.yamldatabase/config/base/kustomization.yamldatabase/config/base/metrics_service.yamldatabase/config/default/kustomization.yamldatabase/config/default/manager_metrics_patch.yamldatabase/config/default/namespace.yamldatabase/config/manager/manager.yamldatabase/config/overlays/operator-config/kustomization.yamldatabase/config/overlays/operator-config/manager_config_patch.yamldatabase/config/overlays/operator-config/operator_config.yamldatabase/go.moddatabase/internal/config/defaults.godatabase/internal/config/flags.godatabase/internal/config/load.godatabase/internal/config/load_test.godatabase/internal/config/namespace.godatabase/internal/config/namespace_test.godatabase/internal/config/types.godatabase/internal/config/validate.godatabase/internal/config/validate_test.godatabase/internal/controller/controller_test_helpers_test.godatabase/internal/controller/dbinstance_controller.godatabase/internal/credentials/resolver.godatabase/internal/credentials/resolver_test.godatabase/internal/ensure/defaults.godatabase/internal/ensure/defaults_test.godatabase/internal/ensure/dependencies.godatabase/internal/ensure/health.godatabase/internal/ensure/monitoring.godatabase/internal/ensure/preflight.godatabase/internal/ensure/preflight_test.godatabase/internal/ensure/resize.godatabase/internal/ensure/steps_test_helpers_test.godatabase/internal/ensure/vm.godatabase/internal/gateway/gateway.godatabase/internal/gateway/gateway_test.godatabase/internal/harvester/typed_client.godatabase/internal/harvester/typed_client_test.godatabase/internal/resource/builder_test.godatabase/internal/resource/servicemonitor.go
💤 Files with no reviewable changes (1)
- database/config/default/manager_metrics_patch.yaml
| images: | ||
| - name: ghcr.io/yohansenanayake/dbaas-controller | ||
| newName: ghcr.io/yohansenanayake/dbaas-controller | ||
| newTag: dev-b658cf2-202607290025 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use an image tag available today.
This tag is dated July 29, 2026, but today is July 28, 2026. Applying the overlay before that image is published will leave the manager in ImagePullBackOff. Replace it with an existing immutable tag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/config/overlays/operator-config/kustomization.yaml` around lines 8 -
11, Update the dbaas-controller image entry in kustomization.yaml to use an
already-published immutable tag dated no later than July 28, 2026, replacing
dev-b658cf2-202607290025. Keep the existing image repository unchanged.
| if err := validateBindAddress("server.health.bindAddress", c.Server.Health.BindAddress, false); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does controller-runtime manager Options.HealthProbeBindAddress support "0" to disable health probes?
💡 Result:
Yes, controller-runtime manager supports using "0" or an empty string ("") in HealthProbeBindAddress to disable health probes [1][2][3]. The official documentation and source code explicitly state that HealthProbeBindAddress is the TCP address the controller binds to for serving health probes, and setting it to "0" or "" will disable this functionality [1][2][4]. This is implemented in the internal defaultHealthProbeListener function, which returns nil for the listener when the address is set to "0" or "", effectively preventing the health probe server from starting [1][2][3].
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go
- 2: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.24.1/pkg/manager/manager.go
- 3: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.23.3/pkg/manager/manager.go
- 4: https://github.com/kubernetes-sigs/controller-runtime/blob/304027bcbe4b3f6d582180aec5759eb4db3f17fd/pkg/manager/manager.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files and refs =="
fd -a 'go\.mod|validate\.go' . | sed 's#^\./##'
echo
rg -n 'HealthProbeBindAddress|HealthProbe|health|bindAddress|go\.module|github.com/sigs.k8s.io/controller-runtime|sigs.k8s.io/controller-runtime' -S --glob '!vendor' . | head -200
echo
echo "== validate.go relevant section =="
if [ -f database/internal/config/validate.go ]; then
nl -ba database/internal/config/validate.go | sed -n '1,130p'
fi
echo
echo "== go.mod controller-runtime versions =="
if [ -f go.mod ]; then rg 'controller-runtime|sigs.k8s.io/controller-runtime' go.mod; fi
echo
if [ -f go.sum ]; then rg 'controller-runtime|sigs.k8s.io/controller-runtime' go.sum | head -20; fiRepository: wso2/open-cloud-datacenter
Length of output: 21910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== validate.go relevant section =="
awk '{printf "%6d %s\n", NR, $0}' database/internal/config/validate.go | sed -n '1,120p'
echo
echo "== database go.mod =="
awk '{printf "%6d %s\n", NR, $0}' database/go.mod
echo
echo "== validate helper implementation and tests =="
python3 - <<'PY'
from pathlib import Path
p = Path('database/internal/config/validate.go')
s = p.read_text()
for needle in ['func validateBindAddress', 'func Validate']:
idx = s.find(needle)
print(f'--- {needle} at {idx} ---')
print(s[idx:idx+800].split('\n---')[0][:800] if idx != -1 else 'not found')
print('contains allowDisabled check:', 'allowDisabled' in s)
PY
echo
echo "== deterministic parser for validateBindAddress-like behavior =="
python3 - <<'PY'
from pathlib import Path
p = Path('database/internal/config/validate.go')
text = p.read_text()
print(text[:text.find('\npackage ')].count('\n'))
import re
m = re.search(r'func validateBindAddress\([^)]*\) string, value string, allowDisabled bool[^{]+{(?P<body>.*?)(?=\n[}\t ]\n|func |\Z)', text, re.S)
if not m:
m = re.search(r'func validateBindAddress\([^)]*\) string, value string, allowDisabled bool[^{]+{(?P<body>.*?)(?=\nfunc |\Z)', text, re.S)
body = m.group('body') if m else text
print(body)
def behavior(body):
# Extract conditions around value == "0" and allowDisabled
out = []
for part in body.split('\n'):
if 'value' in part or 'allowDisabled' in part or 'strconv' in part or 'net' in part:
out.append(part.strip())
return out
print('conditions:', behavior(body))
PYRepository: wso2/open-cloud-datacenter
Length of output: 23241
🌐 Web query:
controller-runtime v0.20.4 pkg manager healthProbeBindAddress "0" HealthProbeBindAddress
💡 Result:
In controller-runtime v0.20.4, the HealthProbeBindAddress field within the manager.Options struct (part of the sigs.k8s.io/controller-runtime/pkg/manager package) specifies the TCP address that the controller manager uses to bind and serve health probes [1][2]. Setting this field to "0" or an empty string ("") explicitly disables the serving of health probes [1][3]. When a valid address (e.g., ":8081" or "0.0.0.0:8081") is provided, the manager uses the defaultHealthProbeListener function to create a TCP listener on that address [1][2]. Because this mechanism creates a standard TCP listener, it is common to use tools like nc (netcat) to verify connectivity if HTTP requests to endpoints like /healthz or /readyz do not behave as expected [4]. Note that while the configuration sets the binding address, custom health check logic can be added to the manager using the AddHealthzCheck and AddReadyzCheck methods [4].
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go
- 2: https://github.com/kubernetes-sigs/controller-runtime/blob/229c3c357d9e6b07c3d6774010c35161a82b08f9/pkg/manager/manager.go
- 3: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.23.3/pkg/manager/manager.go
- 4: Health checks return 404 code kubernetes-sigs/controller-runtime#2360
🌐 Web query:
controller-runtime v0.20.4 pkg manager manager.go HealthProbeBindAddress disable "0"
💡 Result:
In controller-runtime v0.20.4, you can disable the health probe server by setting the HealthProbeBindAddress option to either "0" or an empty string ("") [1][2]. The default health probe listener function, defined within the package manager, explicitly checks for these values to determine whether to skip creating the listener [1][2]. If the address is set to "0" or "", the function returns nil for the listener, effectively disabling the serving of health probes [1][3][2]. Source code context: The relevant logic in pkg/manager/manager.go is implemented as follows: func defaultHealthProbeListener(addr string) (net.Listener, error) { if addr == "" || addr == "0" { return nil, nil } ln, err:= net.Listen("tcp", addr) //... return ln, nil } This behavior ensures that when "0" is provided, the controller manager does not attempt to bind to any TCP port for health checks [1][4].
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go
- 2: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.24.1/pkg/manager/manager.go
- 3: https://github.com/kubernetes-sigs/controller-runtime/blob/c7a98aa706379c4e5c79ea675c7f333192677971/pkg/manager/manager.go
- 4: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager_test.go
🌐 Web query:
github.com kubernetes-sigs controller-runtime v0.20.4 pkg manager defaultHealthProbeListener "0"
💡 Result:
In the Kubernetes controller-runtime library, including version v0.20.4, the defaultHealthProbeListener is a helper function defined within the pkg/manager/manager.go file [1][2][3]. Its primary purpose is to create a TCP listener for health probes [1][2]. The function checks the provided address (addr) and returns early—effectively disabling the health probe server—if the address is an empty string ("") or the string "0" [1][2][3]. The implementation is as follows: func defaultHealthProbeListener(addr string) (net.Listener, error) { if addr == "" || addr == "0" { return nil, nil } ln, err:= net.Listen("tcp", addr) if err!= nil { return nil, fmt.Errorf("error listening on %s: %w", addr, err) } return ln, nil } When a manager is initialized, the HealthProbeBindAddress option uses this logic to determine whether to bind to a specific address or disable the health probe server entirely [1][2]. If a user sets the HealthProbeBindAddress to "0" or leaves it as the default (empty string), the manager will not start a health probe listener [1][2].
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go
- 2: https://github.com/kubernetes-sigs/controller-runtime/blob/304027bcbe4b3f6d582180aec5759eb4db3f17fd/pkg/manager/manager.go
- 3: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.24.1/pkg/manager/manager.go
🌐 Web query:
controller-runtime v0.18.0 v0.19.0 v0.20.4 HealthProbeBindAddress defaultHealthProbeListener
💡 Result:
In controller-runtime, HealthProbeBindAddress is a configuration option in the manager's Options struct used to specify the TCP address where the manager serves health probes (such as liveness and readiness checks) [1][2][3]. The function defaultHealthProbeListener is the internal implementation used to create the listener for these probes [1][2][4]. Its behavior is consistent across versions v0.18.0, v0.19.0, and v0.20.4 [1][2][4]: 1. If HealthProbeBindAddress is set to an empty string "" or "0", the function returns nil, nil, effectively disabling the health probe server [1][2][5]. 2. Otherwise, it attempts to open a TCP listener on the provided address using net.Listen("tcp", addr) [1][2][4]. If the bind fails (e.g., due to an invalid address or the port already being in use), it returns an error [1][2][5]. The manager uses this listener to expose health endpoints, which default to /healthz (liveness) and /readyz (readiness) [1][2]. Developers typically set HealthProbeBindAddress (e.g., :8081) in the manager's options, and the manager automatically falls back to defaultHealthProbeListener if a custom listener is not provided [1][2][5].
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go
- 2: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.24.1/pkg/manager/manager.go
- 3: https://github.com/kubernetes-sigs/controller-runtime/blob/v0.14.6/pkg/manager/manager.go
- 4: https://github.com/kubernetes-sigs/controller-runtime/blob/cd65cb25d314f40a329a688f4714fe3282589e97/pkg/manager/manager.go
- 5: https://github.com/kubernetes-sigs/controller-runtime/blob/c7a98aa706379c4e5c79ea675c7f333192677971/pkg/manager/manager.go
Allow disabling health probe binding with "0" for server.health.bindAddress.
HealthProbeBindAddress in controller-runtime treats "0" as disable, and database/cmd/main.go passes this value directly to mgr.Options. Since validateBindAddress(..., false) rejects "0" here, admins cannot use the supported disable sentinel without this validation error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/internal/config/validate.go` around lines 37 - 39, Update the
validateBindAddress call for server.health.bindAddress in config validation to
allow the "0" disable sentinel by enabling the corresponding acceptance flag.
Preserve the existing validation and error propagation for all other
bind-address values.
| return ensure.Dependencies{ | ||
| Client: r.Client, | ||
| Harvester: r.Harvester, | ||
| Recorder: r.Recorder, | ||
| GrafanaBaseURL: r.GrafanaBaseURL, | ||
| OperatorNamespace: r.operatorNamespace(), | ||
| OperatorNamespace: operatorNamespace, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pass configured dependencies through this test helper.
Production wiring supplies DatabaseDefaults, InstanceClasses, and Monitoring, but resetEnsureRunner drops them here and silently exercises built-in fallbacks instead. Controller tests cannot validate configured reconciliation behavior.
Proposed fix
GrafanaBaseURL: r.GrafanaBaseURL,
OperatorNamespace: operatorNamespace,
+ DatabaseDefaults: r.DatabaseDefaults,
+ InstanceClasses: r.InstanceClasses,
+ Monitoring: r.Monitoring,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ensure.Dependencies{ | |
| Client: r.Client, | |
| Harvester: r.Harvester, | |
| Recorder: r.Recorder, | |
| GrafanaBaseURL: r.GrafanaBaseURL, | |
| OperatorNamespace: r.operatorNamespace(), | |
| OperatorNamespace: operatorNamespace, | |
| return ensure.Dependencies{ | |
| Client: r.Client, | |
| Harvester: r.Harvester, | |
| Recorder: r.Recorder, | |
| GrafanaBaseURL: r.GrafanaBaseURL, | |
| OperatorNamespace: operatorNamespace, | |
| DatabaseDefaults: r.DatabaseDefaults, | |
| InstanceClasses: r.InstanceClasses, | |
| Monitoring: r.Monitoring, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/internal/controller/controller_test_helpers_test.go` around lines 49
- 54, Update resetEnsureRunner to pass the configured DatabaseDefaults,
InstanceClasses, and Monitoring dependencies from the runner into
ensure.Dependencies, matching production wiring and avoiding fallback values in
controller tests.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (1)
database/internal/ensure/defaults_test.go (1)
48-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a non-default configuration.
All updated cases pass built-in defaults, so they cannot detect
immutableDriftWithDefaultsignoring its supplied configuration. Add a case with custom database defaults and omitted spec fields that matchAppliedSpec.Also applies to: 74-76, 96-98, 122-124, 150-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/internal/ensure/defaults_test.go` around lines 48 - 50, Add test cases covering custom database defaults passed to immutableDriftWithDefaults, with omitted spec fields matching the corresponding AppliedSpec values and no reported drift. Update the related cases at the referenced assertions to use the custom configuration rather than only operatorconfig.Default().DatabaseDefaults, while preserving existing default-configuration coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@database/config/base/cert_metrics_manager_patch.yaml`:
- Around line 11-14: Replace the stale --metrics-cert-path argument in the
metrics server container args with the registered observability.metrics.tls.*
flag for the mounted certificate directory, preserving the existing
/tmp/k8s-metrics-server/metrics-certs path.
In `@database/config/overlays/operator-config/kustomization.yaml`:
- Around line 8-11: Update the dbaas-controller image entry in
kustomization.yaml to use an already-published immutable tag dated no later than
July 28, 2026, replacing dev-b658cf2-202607290025. Keep the existing image
repository unchanged.
In `@database/internal/config/validate.go`:
- Around line 37-39: Update the validateBindAddress call for
server.health.bindAddress in config validation to allow the "0" disable sentinel
by enabling the corresponding acceptance flag. Preserve the existing validation
and error propagation for all other bind-address values.
In `@database/internal/controller/controller_test_helpers_test.go`:
- Around line 49-54: Update resetEnsureRunner to pass the configured
DatabaseDefaults, InstanceClasses, and Monitoring dependencies from the runner
into ensure.Dependencies, matching production wiring and avoiding fallback
values in controller tests.
---
Nitpick comments:
In `@database/internal/ensure/defaults_test.go`:
- Around line 48-50: Add test cases covering custom database defaults passed to
immutableDriftWithDefaults, with omitted spec fields matching the corresponding
AppliedSpec values and no reported drift. Update the related cases at the
referenced assertions to use the custom configuration rather than only
operatorconfig.Default().DatabaseDefaults, while preserving existing
default-configuration coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cf9de88-99b6-4c68-abc6-66cb539b4bd3
⛔ Files ignored due to path filters (1)
database/go.sumis excluded by!**/*.sum
📒 Files selected for processing (42)
database/README.mddatabase/cmd/main.godatabase/config/base/cert_metrics_manager_patch.yamldatabase/config/base/kustomization.yamldatabase/config/base/metrics_service.yamldatabase/config/default/kustomization.yamldatabase/config/default/manager_metrics_patch.yamldatabase/config/default/namespace.yamldatabase/config/manager/manager.yamldatabase/config/overlays/operator-config/kustomization.yamldatabase/config/overlays/operator-config/manager_config_patch.yamldatabase/config/overlays/operator-config/operator_config.yamldatabase/go.moddatabase/internal/config/defaults.godatabase/internal/config/flags.godatabase/internal/config/load.godatabase/internal/config/load_test.godatabase/internal/config/namespace.godatabase/internal/config/namespace_test.godatabase/internal/config/types.godatabase/internal/config/validate.godatabase/internal/config/validate_test.godatabase/internal/controller/controller_test_helpers_test.godatabase/internal/controller/dbinstance_controller.godatabase/internal/credentials/resolver.godatabase/internal/credentials/resolver_test.godatabase/internal/ensure/defaults.godatabase/internal/ensure/defaults_test.godatabase/internal/ensure/dependencies.godatabase/internal/ensure/health.godatabase/internal/ensure/monitoring.godatabase/internal/ensure/preflight.godatabase/internal/ensure/preflight_test.godatabase/internal/ensure/resize.godatabase/internal/ensure/steps_test_helpers_test.godatabase/internal/ensure/vm.godatabase/internal/gateway/gateway.godatabase/internal/gateway/gateway_test.godatabase/internal/harvester/typed_client.godatabase/internal/harvester/typed_client_test.godatabase/internal/resource/builder_test.godatabase/internal/resource/servicemonitor.go
💤 Files with no reviewable changes (1)
- database/config/default/manager_metrics_patch.yaml
🛑 Comments failed to post (1)
database/config/base/cert_metrics_manager_patch.yaml (1)
11-14: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash ast-grep outline database/internal/config/flags.go --items all rg -n -C2 'metrics-cert-path|observability\.metrics\.tls\.certDir|certDir' \ database/internal/config/flags.go database/cmd/main.goRepository: wso2/open-cloud-datacenter
Length of output: 1796
🏁 Script executed:
#!/bin/bash cat -n database/internal/config/flags.go printf '\n--- patch file ---\n' cat -n database/config/base/cert_metrics_manager_patch.yaml printf '\n--- main setup section ---\n' sed -n '80,135p' database/cmd/main.go | cat -nRepository: wso2/open-cloud-datacenter
Length of output: 8779
Replace the stale metrics certificate flag.
--metrics-cert-pathis not a registered CLI flag; the manager parses arguments fromdatabase/internal/config/flags.go, whose current metrics TLS options useobservability.metrics.tls.*. Use that flag for the mounted cert directory.Proposed fix
- value: --metrics-cert-path=/tmp/k8s-metrics-server/metrics-certs + value: --observability.metrics.tls.certDir=/tmp/k8s-metrics-server/metrics-certs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Add the --metrics-cert-path argument for the metrics server - op: add path: /spec/template/spec/containers/0/args/- value: --observability.metrics.tls.certDir=/tmp/k8s-metrics-server/metrics-certs🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/config/base/cert_metrics_manager_patch.yaml` around lines 11 - 14, Replace the stale --metrics-cert-path argument in the metrics server container args with the registered observability.metrics.tls.* flag for the mounted certificate directory, preserving the existing /tmp/k8s-metrics-server/metrics-certs path.
2 participants
Summary
Centralizes application-level operator configuration in a typed, validated hierarchy using
nil-go/konf.The default installation continues to work without a configuration ConfigMap, while an optional Kustomize overlay provides centralized file configuration at
/etc/dbaas/config.json.Closes #244 .
Related: Discussion #240.
Changes
built-in defaults < configuration file < environment variables < explicit flags.DBAAS_environment-variable mapping./etc/dbaas/config.jsonpath.dbaas-system.POD_NAMESPACE.operator.namespacefrom runtime configuration to prevent deployment and runtime namespace drift.Testing
go test ./...bin/kustomize build config/basebin/kustomize build config/defaultbin/kustomize build config/overlays/operator-configdbaas-system-v2.git diff --checkAll checks passed.
Checklist
Summary by CodeRabbit