* 🐛 Apply TLS profile to spoke agent serving endpoints
The spoke agent binaries (klusterlet-agent, registration-agent, work-agent)
receive --tls-min-version and --tls-cipher-suites flags from the klusterlet
operator via deployment template rendering, but do not wire them to their own
library-go GenericAPIServer serving endpoint on port 8443.
Add ApplyTLSToCommand to all three spoke agent command constructors
(NewKlusterletAgentCmd, NewRegistrationAgent, NewWorkAgent) so the TLS flags
are consumed by the PersistentPreRunE hook and applied to the serving config.
This matches the pattern already used by all hub component commands
(registration-controller, placement, work, addon-manager, grpc-server).
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* 🐛 Apply TLS profile to operator serving endpoints via ConfigMap
The cluster-manager operator (hub) and klusterlet operator (spoke) serve a
health/metrics endpoint on port 8443 via library-go's GenericAPIServer. Unlike
the hub controllers and spoke agents they manage, these operator binaries do not
receive --tls-min-version and --tls-cipher-suites flags from their deployment
manifests -- no external component injects those flags into the operator pods.
Instead, the operators have direct access to the ocm-tls-profile ConfigMap in
their namespace at startup:
- The cluster-manager operator reads it in RunClusterManagerOperator via
StartTLSConfigMapWatcher and calls os.Exit(0) on changes to restart.
- The klusterlet operator has a tls-profile-sync sidecar that writes the
ConfigMap from the local OCP APIServer TLS profile and triggers pod restarts.
Add ApplyTLSFromConfigMapToCommand to both operator commands. This installs a
PersistentPreRunE hook that reads the ocm-tls-profile ConfigMap once (using an
in-cluster kube client) before library-go's StartController creates the server,
writes a minimal GenericOperatorConfig YAML to /tmp, and sets --config to point
at it. This ensures the server's TLS config is set correctly from the first
request, using the same restart-on-change mechanism already in place.
The ConfigMap is optional: if not found (upstream deployments without the ACM
sidecar) or if the in-cluster config is unavailable (local dev), the hook is a
no-op and library-go defaults apply.
Also add ApplyTLSToCommand call-site comments to the three spoke agent commands
explaining the difference: agents receive TLS flags from their deployment
manifests (injected by the klusterlet operator), while the operator itself reads
the ConfigMap directly.
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* feat: add NetworkPolicies for open-cluster-management-agent namespace
Ship 4 NetworkPolicy manifests with the klusterlet operator to restrict
ingress/egress in the agent namespace, meeting CIS Kube benchmark 5.3.2
requirements.
Policies added:
- default-deny-all: baseline deny for all ingress/egress
- allow-dns-and-api: DNS egress (OpenShift/kube-dns) + ports-only API
server egress (TCP 443/6443)
- klusterlet: operator egress to intra-namespace, addon namespace, and
kubernetes.default.svc
- klusterlet-agent: agent egress to kubernetes.default.svc, hub webhooks,
and intra-namespace
Also adds:
- RBAC: networking.k8s.io/networkpolicies permissions in klusterlet
ClusterRole
- Cleanup: *networkingv1.NetworkPolicy case in CleanUpStaticObject and
GenerateRelatedResource
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* feat: add NetworkPolicies feature gate and fix ingress rules
- Add NetworkPolicies feature gate (disabled by default) following the
existing AboutAPIEnabled/ClusterProperty pattern
- Filter operator-internal feature gates before ConvertToFeatureGateFlags
to avoid invalid agent CLI flags and ValidFeatureGates condition issues
- Conditionally apply NP manifests in both reconcile() and clean() based
on the feature gate
- Fix missing intra-namespace ingress rules in klusterlet and
klusterlet-agent NetworkPolicies (CodeRabbit review feedback)
- Revert test counts to pre-NP values and add
TestSyncDeployWithNetworkPolicies for the feature-enabled path
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* fix: address CodeRabbit nitpicks - import grouping and defensive slice copy
- Move featuregate import to correct alphabetical position in its group
- Use defensive slice copy (append into new slice) instead of direct
assignment from package-level var before appending
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* fix: update OLM CSV with NetworkPolicy RBAC permissions
The ClusterServiceVersion file needs to include the networking.k8s.io
networkpolicies RBAC rules added to the klusterlet ClusterRole.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* test: enable NetworkPolicies feature gate in integration and e2e tests
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* test: update relatedResources counts for NetworkPolicy resources
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* fix: update expected registration deployment args count for RegistrationConfiguration defaults
RegistrationConfiguration triggers kubebuilder defaults (KubeAPIQPS=50,
KubeAPIBurst=100), adding --kube-api-qps and --kube-api-burst args.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
* fix: clarify relatedResources count comment arithmetic
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Mihir Lele <mlele@redhat.com>
---------
Signed-off-by: Mihir Lele <mlele@redhat.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Map GET/POST debug requests to get/create SAR verbs and return 403 on
denial. Reject unsupported HTTP methods with 405 instead of treating
them as GET. Update the GET RBAC integration test Role to grant get.
Also free unused disk on e2e runners before image builds to reduce
"no space left on device" flakes.
Fixesopen-cluster-management-io/ocm#1622
Signed-off-by: Roke Jung <roke@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Add configurable image pull secrets to cluster-manager
Allow referencing existing pull secrets via images.imagePullSecrets in the
cluster-manager Helm chart. Propagate the configured secret name to hub
components through the operator --image-pull-secret-name flag, including
cluster importer renderers and registration controller RBAC.
Signed-off-by: Enin <enin.kaduk@docker.com>
* Require .name if imagePullSecrets is set, trim ws in normalization of imagePullSecrets
Signed-off-by: Enin <enin.kaduk@docker.com>
* update-csv
Signed-off-by: Mike Ng <ming@redhat.com>
---------
Signed-off-by: Enin <enin.kaduk@docker.com>
Signed-off-by: Mike Ng <ming@redhat.com>
Co-authored-by: Mike Ng <ming@redhat.com>
* fix: preserve config reference ordering in cmainstallprogression controller
Replace sets.Set[ConfigReferent] with []ConfigReferent in setInstallProgression
and overrideConfigMapByAddOnConfigs. Iterating a set (Go map) produces
nondeterministic ordering when a placement has multiple configs of the same GVK,
causing spurious status patches on every reconcile and unstable config ordering
downstream to MCA status.
The fix preserves the insertion order from the CMA spec's placement configs,
matching the pattern already used by overrideConfigMapByAddOnConfigs in
pkg/addon/controllers/addonconfiguration/graph.go.
Deduplication is maintained via a containsConfigReferent helper (linear scan),
consistent with the addonconfiguration controller's containsConfig method.
Fixes: https://github.com/open-cluster-management-io/ocm/issues/1609
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add integration test for multi-same-GVK config reference ordering
Adds an integration test that verifies the full pipeline when a CMA placement
has two configs of the same GVK. Asserts that both CMA status
InstallProgressions and MCA status ConfigReferences reflect the configs in
the order specified in the CMA placement spec (config-first before config-second),
not in a nondeterministic order.
This exercises the full controller chain:
- cmainstallprogression controller: CMA spec -> CMA status
- addonconfiguration controller: CMA status -> MCA status
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
PlacementDebugServer previously returned HTTP 200 for all error responses
because reportErr never called WriteHeader. Classify errors as 400/401/403/404/500,
reject incomplete debug paths without a namespace, and add unit test coverage
for all reachable reportErr paths.
Fixesopen-cluster-management-io/ocm#1592
Signed-off-by: Roke Jung <roke@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* chore: upgrade to go 1.26
Upgrades Go version from 1.25 to 1.26.
Changes:
- go.mod — go directive updated to 1.26.0
- .github/workflows/*.yml — GO_VERSION updated to 1.26 (5 workflows)
- build/Dockerfile.* — base image updated to golang:1.26-bookworm (5 Dockerfiles)
- pkg/placement/controllers/scheduling/scheduling_controller.go — fix
pre-existing printf format mismatch (%q -> %d for int args) surfaced
by stricter Go 1.26 linter
Relates to: https://github.com/open-cluster-management-io/ocm/issues/1555
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* fix: pin engineerd/setup-kind to working commit SHA
Pin engineerd/setup-kind to commit SHA ecfad61750951586a9ef973db567df1d28671bdc
which is the tip of the v0.6.2 branch and includes the required dist/
build artifact. The v0.6.2 tag currently resolves to a different commit
that is missing dist/main/index.js, causing e2e CI to fail.
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
The CertRotationController only included the short SVC hostname
(service.namespace.svc) in the serving certificate SANs. Clients
resolving via the full cluster.local FQDN get a TLS verification
failure because the name doesn't match. Add the .svc.cluster.local
form so both resolve correctly.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-authored-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* CMA converted from v1alpha1 to v1beta1, ignore name=addonv1beta1.ReservedNoDefaultConfigName
Fixes: https://github.com/open-cluster-management-io/ocm/issues/1526
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add sentinel value filtering tests for cmainstallprogression
Add test cases to verify that the ReservedNoDefaultConfigName sentinel
value is properly filtered out from both DefaultConfigReferences and
InstallProgressions status fields.
These tests ensure that v1alpha1 CMAs with supportedConfigs but no
defaultConfig don't leak the sentinel value into status, which would
cause addons using older addon-framework versions to fail.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Add service-CA certificate support for PlacementDebugServer
When the PlacementDebugServer feature gate is enabled, inject a
serving-cert annotation into the placement service and mount the
resulting TLS secret into the debug-server container. On OpenShift,
the service-serving-cert controller creates a CA-signed certificate
automatically. On non-OpenShift, optional: true allows the pod to
start and library-go falls back to self-signed certificates.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Rename fields to PlacementAnnotations and PlacementServingCertSecret
Scope field names to Placement per review feedback, since these
are only used for the placement service and extending to other
services would require separate fields.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Replace OCP annotation with CertRotationController for PlacementDebugServer TLS
Replaces the OpenShift-specific serving-cert-secret-name annotation with the
OCM-native CertRotationController to provision the PlacementDebugServer's TLS
serving certificate. Follows the existing GRPC conditional target pattern:
the placement-debug-serving-cert target is added/removed based on the
PlacementDebugServer feature gate.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix misleading error messages and import ordering
Correct error messages in feature-disabled cleanup paths to accurately
state the operation (secret deletion with feature disabled) instead of
implying a deleted ClusterManager. Also move ocmfeature import into the
open-cluster-management.io group where it belongs.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix RBAC and gofmt for PlacementDebugServer cert rotation
Add placement-debug-serving-cert to the cluster-manager ClusterRole
resourceNames allowlist and fix gofmt alignment in two files.
Root cause of E2E failures: the certRotationController attempts to
delete the placement-debug-serving-cert secret when the feature gate
is disabled (the default). The operator ClusterRole restricts secret
delete/get/update/patch to an explicit resourceNames list. Because
placement-debug-serving-cert was not in that list, the delete call
returned 403 Forbidden — not 404 NotFound. The error handler in
syncOne() only ignores IsNotFound, so 403 caused an early return
before the signing CA and ca-bundle-configmap were ever created.
The clusterManagerController.sync() blocks at line 312 waiting for
ca-bundle-configmap to appear, so ObservedGeneration was never set,
and all four E2E suites timed out in BeforeSuite after 150 seconds.
The gofmt failures were cosmetic: extra alignment spaces in the
PlacementDebugServingCertSecret/PlacementDebugService const block
and the PlacementDebugServerEnabled/PlacementServingCertSecret
struct fields.
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* retrigger CI
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
* retrigger CI
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
---------
Signed-off-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-authored-by: Randy Bruno Piverger <21374229+Randy424@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* 🐛 Fix concurrency bugs in executor cache
- Fix DimensionCaches.remove() using RLock instead of Lock for map
delete operation, which could cause concurrent map read/write panic
- Fix RemoveByHash accessing len(items) without holding the lock
- Fix getCacheItems returning internal map reference, allowing
unsynchronized iteration after lock release; return snapshot copies
- Add early return in updateSARCheckResultToCache for clarity
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🐛 Fix wrong index in clusterRoleEnqueueFu causing missed cache refresh
When a ClusterRole changes, the controller should find RoleBindings
referencing it via the byClusterRole index. It was incorrectly using
the byRole index, which indexes by "namespace/name" for Role refs
and never matches a bare ClusterRole name. This caused executor
caches to not refresh when a ClusterRole was modified, leaving
revoked permissions cached as allowed for up to 10 minutes.
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Make RemoveByHash private as it is only used internally
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
* Add concurrency and index-fix tests for executor cache
- Add concurrent remove/get, getCacheItems, and cleanup tests to verify
race-free behavior with -race detector
- Add TestCacheControllerClusterRoleWithRoleBindingOnly to verify
clusterRoleEnqueueFu uses byClusterRole index for RoleBindings
Signed-off-by: zhujian <jiazhu@redhat.com>
---------
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Copy TLS ConfigMap to addon namespaces in klusterlet operator
Add AddonTLSConfigController that copies the ocm-tls-profile ConfigMap
from the klusterlet operator namespace to addon namespaces (labeled with
addon.open-cluster-management.io/namespace: "true"). This allows addon
agents to read TLS profile settings without cross-namespace RBAC.
The controller mirrors the existing addonsecretcontroller pattern:
- Watches namespaces with the addon label via filtered informer
- Copies ConfigMap data on namespace creation/update
- Deletes target ConfigMap when source is removed
- Skips update when target is already up-to-date
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Fix ConfigMap update to preserve ResourceVersion and add stale-target test
- Reuse existing ConfigMap object on update to preserve ResourceVersion,
preventing optimistic concurrency conflicts
- Add test case for stale target ConfigMap being updated
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
---------
Signed-off-by: zhujian <jiazhu@redhat.com>
Add TLS profile compliance to the gRPC server, completing TLS support
for all hub components. The operator reads the ocm-tls-profile ConfigMap
and injects --tls-min-version and --tls-cipher-suites flags into the
gRPC server deployment, matching the pattern used by all other hub
component deployments.
Changes:
- Add TLS flag injection to gRPC server deployment manifest
- Wire TLS flags from common options to gRPC server via closure
- Call ApplyTLSToCommand for the 8443 health server endpoint
- Apply TLS overrides to the 8090 gRPC port via SDK ApplyTLSFlags
- Update vendored sdk-go with CipherSuites support for gRPC server
- Add unit, controller, and integration tests
Assisted by Claude
Signed-off-by: zhujian <jiazhu@redhat.com>
* feat(placement): split debug controller as standalone service with proper validation
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Qing Hao <qhao@redhat.com>
* feat(placement): make placement service conditional on PlacementDebugServer feature gate
Make placement debug service deployment conditional based on
PlacementDebugServer feature gate to allow users to control
whether to expose the debug endpoint.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Qing Hao <qhao@redhat.com>
---------
Signed-off-by: Qing Hao <qhao@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
* 🌱 Add TLS profile configuration support via flags and ConfigMap
Add pkg/common/tls library to support TLS profile compliance
for OCM components. This enables components to receive TLS
configuration via command-line flags (--tls-min-version and
--tls-cipher-suites) from operators, aligning with the upstream
enhancement proposal for TLS profile configuration.
Key features:
- TLS version and cipher suite parsing from flags or ConfigMap
- ConfigMap-based TLS configuration for operator use
- ConfigMap watcher for operators to detect profile changes
- OpenSSL cipher name mapping to Go crypto/tls constants
- Safe defaults (TLS 1.2) when no configuration provided
Updated pkg/common/options/webhook.go to use TLS library instead
of hardcoded TLS 1.2, enabling webhook components to respect
TLS flags injected by operators.
This is the foundation for OCM TLS profile compliance, keeping
upstream code OpenShift-agnostic while supporting dynamic TLS
configuration.
Related: open-cluster-management-io/enhancements#175
Signed-off-by: Jia Zhu <jiazhu@redhat.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Add TLS ConfigMap watch and restart to cluster-manager operator
Implement ConfigMap-based TLS profile compliance for cluster-manager operator
with hash comparison to prevent infinite restart loops.
Changes:
- Add TLS ConfigMap informer to watch ocm-tls-profile ConfigMap
- Load current TLS config at startup and compute hash
- Add event handlers that compare ConfigMap hash with current hash
- Only restart if ConfigMap content actually differs from current config
- Add comprehensive logging for all scenarios
Scenarios handled:
✅ ConfigMap exists at startup (hash matches) → no restart
✅ ConfigMap created after startup (hash differs) → restart to apply
✅ ConfigMap updated (new hash differs) → restart to apply
✅ ConfigMap deleted (was using it) → restart to use defaults
Leader election behavior:
- This code only runs on the leader pod (due to controllercmd framework)
- Non-leader pods wait idle until they acquire leadership
- New leaders load current ConfigMap state when they start, ensuring latest config
- Only the active leader monitors ConfigMap changes and restarts
🤖 Generated with Claude Code
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Inject TLS config flags into addon-webhook deployment
Implement Case 2 pattern for addon-webhook TLS configuration:
cluster-manager-operator loads TLS config from ConfigMap and injects
it as flags into the addon-webhook deployment.
Changes:
- Add AddonWebhookTLSMinVersion and AddonWebhookTLSCipherSuites fields to HubConfig
- Load TLS config once when creating ClusterManagerController
- Pass TLS config strings as parameters to controller
- Inject --tls-min-version and --tls-cipher-suites flags into addon-webhook deployment template
This approach ensures addon-webhook receives TLS configuration via flags
without needing to watch the ConfigMap itself. When the ConfigMap changes,
cluster-manager-operator restarts, reloads the config, and updates the
deployment with new flags.
🤖 Generated with Claude Code
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Log TLS min version and cipher suites on startup
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Move TLS library to sdk-go and update vendor dependencies
Relocates TLS config and cipher helpers from pkg/common/tls into the
vendored open-cluster-management.io/sdk-go/pkg/tls package, adds a
generic watcher utility, and updates all import references accordingly.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Inject TLS flags into all hub component deployments
Extend TLS flag injection from addon-webhook-only to all seven
hub deployments managed by cluster-manager-operator:
Manifests (operator → deployment args):
- Rename HubConfig.AddonWebhookTLS* → TLS* so the same fields
drive all deployments rather than only the addon webhook
- Add {{- if .TLSMinVersion }} blocks to all six remaining
deployment manifests (registration/work/placement controllers
and registration/work webhook servers)
Controller binaries (registration, work, placement, addon-manager):
- Add --tls-min-version and --tls-cipher-suites flags to the
common Options struct so the binaries accept the injected flags
without failing; the flags are stored for future use
Note: library-go's NewCommandWithContext uses cmd.Run (not RunE),
so there is no clean programmatic hook to inject TLS into the 8443
health server without bypassing library-go's own boilerplate
(signal handling, log init, profiling). Upstream library-go also
has no native TLS configuration API on ControllerCommandConfig or
ControllerBuilder. The 8443 health server defaults to TLS 1.2 via
SetRecommendedHTTPServingInfoDefaults; configuring it further
requires an upstream library-go enhancement.
Webhook binaries already fully support these flags via WebhookOptions;
no binary changes are needed there.
Signed-off-by: Jian Zhu <zhujian@redhat.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Wire --tls-min-version to library-go 8443 health server via WithServingTLSConfig
Now that library-go has WithServingTLSConfig (ServingMinTLSVersion /
ServingCipherSuites fields + injection in StartController before
WithServer is called), wire the --tls-min-version and
--tls-cipher-suites flags from Options into it.
ApplyTLSToCommand installs a PersistentPreRunE hook that calls
CmdConfig.WithServingTLSConfig after cobra flag parsing completes.
PersistentPreRunE runs before cmd.Run, so all library-go boilerplate
(signal handling, logging, profiling) is preserved - unlike the
previous approach of replacing RunE which silently bypassed it.
Uses go mod replace → /Users/jiazhu/go/src/github.com/openshift/library-go
for local development/testing; replace directive to be removed once the
library-go PR is merged and vendored.
Signed-off-by: Jian Zhu <zhujian@redhat.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Switch to --config file for controller 8443 TLS configuration
Replace the WithServingTLSConfig approach with library-go's native
--config flag mechanism:
ApplyTLSToCommand now installs a PersistentPreRunE hook that:
1. Writes a minimal GenericOperatorConfig YAML to a temp file under
/tmp (which is mounted as an emptyDir in all hub controller
deployments, so writing is safe even with readOnlyRootFilesystem)
2. Sets --config to point at the temp file before cmd.Run executes
All library-go boilerplate in cmd.Run (signal handling, log init,
profiling, basicFlags.Validate) is fully preserved because
PersistentPreRunE runs before Run, not replacing it.
Inside StartController, Config() reads the temp file; the TLS values
survive SetRecommendedHTTPServingInfoDefaults because DefaultString
only sets fields that are currently empty.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Add tests for TLS profile compliance
Unit tests (pkg/common/options):
- TestApplyTLSToCommand: table-driven test covering all flag combinations:
no flags (no-op), min-version only, cipher-suites only, both set,
and --config pre-set by user (injection skipped).
Unit tests (clustermanager_controller):
- TestSyncDeployWithTLSConfig: verifies that when tlsMinVersion /
tlsCipherSuites are set on the controller, the --tls-min-version and
--tls-cipher-suites flags appear in the args of every managed hub
deployment (registration, registration-webhook, placement, work-webhook).
Also verifies the flags are absent when TLS config is not set.
Integration tests (test/integration/operator):
- "should inject tls-min-version into all hub deployments when
ocm-tls-profile ConfigMap exists": creates the ocm-tls-profile
ConfigMap with minTLSVersion=VersionTLS13 in the operator namespace
and verifies all six hub deployments gain --tls-min-version=VersionTLS13
in their container args.
Signed-off-by: Jian Zhu <zhujian@redhat.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Switch TLS cipher suite format from OpenSSL to IANA
Update vendored sdk-go to use IANA cipher suite names (e.g.
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) instead of OpenSSL names
(e.g. ECDHE-RSA-AES128-GCM-SHA256).
IANA is the canonical format used by Go's crypto/tls, the Kubernetes
apiserver --tls-cipher-suites flag, and library-go's ServingInfo.CipherSuites.
Using IANA names end-to-end eliminates the format mismatch that caused
library-go's 8443 health server to reject cipher suite names written by
ApplyTLSToCommand.
The ocm-tls-profile ConfigMap now accepts IANA names only. The downstream
tls-profile-sync sidecar is responsible for converting OpenShift
TLSSecurityProfile (OpenSSL-style) names to IANA before writing the ConfigMap.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* 🌱 Fix TLS ConfigMap test: create ConfigMap before operator startup
The previous test created ocm-tls-profile ConfigMap after the operator
started, which triggered the watcher's hash-change detection and called
os.Exit(0), killing the test process. Move the test into a dedicated
Describe with BeforeEach that creates the ConfigMap before starting the
operator so the watcher seeds its hash at startup and no restart is
triggered.
Also add hubWorkControllerDeployment to the tlsDeployments list since
its manifest includes tls-min-version injection.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
---------
Signed-off-by: Jia Zhu <jiazhu@redhat.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
Signed-off-by: Jian Zhu <zhujian@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Move addon api to beta in registration
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* Update conversion e2e tests
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* Add addon v1beta1 to supported type in grpc
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* Fix flaky e2e in addon conversion
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* Set subject for registration configuration when it is not set
Signed-off-by: Jian Qiu <jqiu@redhat.com>
---------
Signed-off-by: Jian Qiu <jqiu@redhat.com>
This commit fixes a security vulnerability where the ManifestWork
validating webhook was not passing the UserInfo.Extra field when
constructing SubjectAccessReview (SAR) requests. This omission could
lead to authorization bypass when external authorization policies
rely on Extra fields (e.g., OIDC claims, department attributes).
The fix adds Extra field conversion logic consistent with the
ManagedCluster webhook implementation and includes comprehensive
test coverage to verify the Extra field is properly propagated.
Fixes#1425🤖 Assisted by Claude Code
Signed-off-by: zhujian <jiazhu@redhat.com>
* Support default mode webhook networking configuration
Signed-off-by: Ben Perry <bhperry94@gmail.com>
* Share common webhook config between hosted and default mode
Signed-off-by: Ben Perry <bhperry94@gmail.com>
* Nest all related bind configuration together
Signed-off-by: Ben Perry <bhperry94@gmail.com>
* Disable surge with hostNetwork to prevent port conflicts
Signed-off-by: Ben Perry <bhperry94@gmail.com>
* Remove dev dependency
Signed-off-by: Ben Perry <bhperry94@gmail.com>
* Set defaults in one place
Signed-off-by: Ben Perry <bhperry94@gmail.com>
---------
Signed-off-by: Ben Perry <bhperry94@gmail.com>
The TestHubTimeoutController_Sync test was failing intermittently in CI
due to timing issues with time.Sleep() and real-time execution overhead.
Changes:
- Removed time.Sleep() dependency that caused flakiness
- Set lease renew time in the past using time.Now().Add(-duration)
to deterministically simulate aged leases
- Made timeout threshold configurable per test case
- Increased safety margin from 2s to 3s in "not timeout" case
(2s lease age vs 5s timeout, previously 1s wait vs 3s timeout)
- Set startTime in the past to bypass the 10s grace period check
that was added to handle stale lease scenarios
This eliminates race conditions in CI environments where execution
overhead could push the test beyond the timeout threshold.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: zhujian <jiazhu@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
The TestNewAgentOptions test was failing in CI because it expected
ComponentNamespace to always be "open-cluster-management-agent", but
NewAgentOptions() reads from /var/run/secrets/kubernetes.io/serviceaccount/namespace
when running in a Kubernetes pod (which exists in CI environment).
Updated the test to accept either the default value (when running locally)
or the actual pod namespace (when running in CI), while ensuring the
namespace is never empty.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: zhujian <jiazhu@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 2m4s
Post / coverage (push) Failing after 7m14s
Post / images (amd64, placement) (push) Failing after 47s
Post / images (amd64, registration-operator) (push) Failing after 40s
Post / images (amd64, work) (push) Failing after 41s
Post / images (amd64, addon-manager) (push) Failing after 7m50s
Post / images (arm64, addon-manager) (push) Failing after 42s
Post / images (arm64, registration) (push) Failing after 41s
Post / images (arm64, registration-operator) (push) Failing after 39s
Post / images (arm64, work) (push) Failing after 44s
Post / images (arm64, placement) (push) Failing after 7m13s
Post / images (amd64, registration) (push) Failing after 12m57s
Post / image manifest (addon-manager) (push) Has been skipped
Post / image manifest (placement) (push) Has been skipped
Post / image manifest (registration) (push) Has been skipped
Post / image manifest (registration-operator) (push) Has been skipped
Post / image manifest (work) (push) Has been skipped
Post / trigger clusteradm e2e (push) Has been skipped
Close stale issues and PRs / stale (push) Successful in 7s
* ✨ Support token-based authentication for template addons
This change enables template type addons to work with both CSR-based
and token-based authentication through dynamic subject binding.
Changes:
- Modified createPermissionBinding() to extract dynamic subjects from
addon.Status.Registrations instead of using hardcoded groups
- Added buildSubjectsFromRegistration() helper to extract user/groups
from registration status
- Returns SubjectNotReadyError when subjects not ready (enables retry)
- Removed clusterAddonGroup() function (no longer needed)
- Updated addon-framework dependency to v1.2.0 for SubjectNotReadyError
- Added comprehensive tests for buildSubjectsFromRegistration
- Updated test helpers to include registration status with proper subjects
The implementation now supports:
- CSR-based authentication (existing)
- Token-based authentication (new)
- Any future authentication method that populates Status.Registrations
Related: https://github.com/elgnay/ocm-enhancements/blob/14af2a2eeb527d30b2c7f196a51e6b7e270be8c8/enhancements/sig-architecture/167-token-based-addon-registration/README.md🤖 Generated with Claude Code
https://claude.com/claude-code
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* test: add unit test for system:authenticated group filtering
Add a test case to verify that buildSubjectsFromRegistration correctly
filters out the system:authenticated group from the list of groups when
building RBAC subjects. This covers the filtering logic in
registration.go lines 560-562.
Also update the expected groups in TestTemplateCSRConfigurationsFunc
to match the implementation that includes both cluster-specific and
addon-wide groups for token-based authentication.
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* feat: add addon-wide group and filter system:authenticated
Add support for addon-wide group in defaultGroups() to support
token-based authentication for template addons. This adds the
system:open-cluster-management:addon:{addonName} group in addition
to the cluster-specific group.
Also add filtering logic in buildSubjectsFromRegistration() to
exclude the system:authenticated group from RBAC subjects, as this
is a special Kubernetes group automatically added to all authenticated
users and should not be explicitly included in RoleBindings.
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* refactor: implement custom CSR approver with flexible org validation
Replace addon-framework's DefaultCSRApprover with a custom implementation
that supports both legacy and new CSR organization structures.
Key changes:
- Implement defaultCSRApprover function that accepts 2 or 3 organization units
- 3 orgs: legacy behavior including system:authenticated group in CSRs
- 2 orgs: new behavior where system:authenticated is filtered out
- Add support for gRPC-based CSR requests by checking CSRUsernameAnnotation
- Validate all required default addon groups are present in CSR
- Add necessary imports: k8s.io/apimachinery/pkg/util/sets and operatorapiv1
This enables backward compatibility while supporting the new token-based
authentication flow where system:authenticated is excluded from CSR orgs
but included in registration configs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* refactor: use addon-framework's updated KubeClientSignerConfigurations
Remove custom implementations and use addon-framework's native functions
which now include system:authenticated group by default.
Changes:
- Remove custom kubeClientSignerConfigurations function
- Remove custom defaultGroups function
- Remove custom defaultCSRApprover function
- Use agent.KubeClientSignerConfigurations from addon-framework
- Use utils.DefaultCSRApprover from addon-framework
- Remove unused imports: k8s.io/apimachinery/pkg/util/sets and operatorapiv1
The addon-framework has been updated to include system:authenticated in
DefaultGroups(), eliminating the need for custom implementations.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
---------
Signed-off-by: zhujian <jiazhu@redhat.com>
Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 20s
Post / images (amd64, placement) (push) Failing after 47s
Post / images (amd64, registration) (push) Failing after 41s
Post / images (amd64, registration-operator) (push) Failing after 45s
Post / images (amd64, work) (push) Failing after 40s
Post / images (arm64, addon-manager) (push) Failing after 44s
Post / images (arm64, placement) (push) Failing after 41s
Post / images (arm64, registration) (push) Failing after 41s
Post / images (arm64, registration-operator) (push) Failing after 41s
Post / images (arm64, work) (push) Failing after 42s
Post / images (amd64, addon-manager) (push) Failing after 7m42s
Post / image manifest (addon-manager) (push) Has been skipped
Post / image manifest (placement) (push) Has been skipped
Post / image manifest (registration) (push) Has been skipped
Post / image manifest (registration-operator) (push) Has been skipped
Post / image manifest (work) (push) Has been skipped
Post / trigger clusteradm e2e (push) Has been skipped
Post / coverage (push) Failing after 9m45s
* check namespace existence and state in clusterprofile lifecycle controller.
Signed-off-by: Morven Cao <lcao@redhat.com>
* optimize the queue key and log for clusterprofile controller.
Signed-off-by: Morven Cao <lcao@redhat.com>
---------
Signed-off-by: Morven Cao <lcao@redhat.com>
* Add watch-based feedback with dynamic informer lifecycle management
Implements dynamic informer registration and cleanup for resources
configured with watch-based status feedback (FeedbackScrapeType=Watch).
This enables real-time status updates for watched resources while
efficiently managing resource lifecycle.
Features:
- Automatically register informers for resources with FeedbackWatchType
- Skip informer registration for FeedbackPollType or when not configured
- Clean up informers when resources are removed from manifestwork
- Clean up informers during applied manifestwork finalization
- Clean up informers when feedback type changes from watch to poll
Implementation:
- Refactored ObjectReader to interface for better modularity
- Added UnRegisterInformerFromAppliedManifestWork helper for bulk cleanup
- Enhanced AvailableStatusController to conditionally register informers
- Updated finalization controllers to unregister informers on cleanup
- Added nil safety checks to prevent panics during cleanup
Testing:
- Unit tests for informer registration based on feedback type
- Unit tests for bulk unregistration and nil safety
- Integration test for end-to-end watch-based feedback workflow
- Integration test for informer cleanup on manifestwork deletion
- All existing tests updated and passing
This feature improves performance by using watch-based updates for
real-time status feedback while maintaining efficient resource cleanup.
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* Fallback to get from client when informer is not synced
Signed-off-by: Jian Qiu <jqiu@redhat.com>
---------
Signed-off-by: Jian Qiu <jqiu@redhat.com>
Mike NgandGitHub
Tesshu FlowerandGitHub
Aman GuptaandGitHub
Artur Shad NikandGitHub
Krishnan K MandGitHub
Vaishnav KaleandGitHub
gauravshinde1729andGitHub
Park GisungandGitHub
shivansh sinhaandGitHub
Jian ZhuandGitHub
Jian QiuandGitHub
Zhiwei YinandGitHub
Wei LiuandGitHub
Ben PerryandGitHub
Navin Chandra RaiandGitHub
Morven CaoandGitHub