* 🐛 fix flaky e2e cleanup and addon conversion test
Pre-delete ManagedClusterAddOns and wait for ManifestWorks before
ManagedCluster deletion in CleanKlusterletRelatedResources.
Merge CMA status update and v1beta1 conversion verify into one
Eventually loop to avoid race with cmaInstallProgressionController.
Fixes#1402
Signed-off-by: bot69dude <nrithvik19@gmail.com>
* Wait for ManagedClusterAddOn deletion in cleanup helper
DeleteAllManagedClusterAddOnsInCluster now waits until all addons
are removed before proceeding, so ManifestWork drain is not racing
with still-active addon controllers.
Addresses CodeRabbit review on #1573
Signed-off-by: bot69dude <nrithvik19@gmail.com>
* Address review: use single MCA API version and drop nested RetryOnConflict
Eventually already retries; listing both converted API versions is redundant.
Signed-off-by: bot69dude <nrithvik19@gmail.com>
---------
Signed-off-by: bot69dude <nrithvik19@gmail.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>
* 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>
* test: add v1beta1 e2e tests for addon lifecycle and install strategy
Add v1beta1 e2e test coverage following the alpha/beta split pattern from
integration tests. Rename framework helper functions to use explicit
V1Alpha1/V1Beta1 suffixes for consistency.
Framework changes (test/framework/managedclusteraddon.go):
- Rename CreateManagedClusterAddOn to CreateManagedClusterAddOnV1Alpha1
- Rename CheckManagedClusterAddOnStatus to CheckManagedClusterAddOnStatusV1Alpha1
- Add CheckManagedClusterAddOnStatusV1Beta1 helper
- Organize functions with Alpha version followed by Beta counterpart
E2E test changes:
- Rename addon_test.go to addon_alpha_test.go (v1alpha1 API)
- Create new addon_test.go with v1beta1 API using V1Beta1 helpers
- Rename addon_install_test.go to addon_install_alpha_test.go (v1alpha1 API)
- Create new addon_install_test.go with v1beta1 API
- Update addon_lease_test.go, addon_token_auth_test.go, addonmanagement_test.go
to use V1Alpha1 helpers
Test coverage (both alpha and v1beta1):
- addon_test.go: Basic ManagedClusterAddOn lifecycle and availability
- addon_install_test.go: ClusterManagementAddOn with placement-based install
strategy, addon annotation syncing
Naming pattern matches integration tests: *_alpha_test.go for v1alpha1,
*_test.go (no suffix) for v1beta1.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add v1beta1 e2e tests for addon lease health check
Add v1beta1 e2e test coverage for addon lease-based health checking
following the alpha/beta split pattern.
Changes:
- Rename addon_lease_test.go to addon_lease_alpha_test.go (v1alpha1 API)
- Create new addon_lease_test.go with v1beta1 API using V1Beta1 helpers
- Update all AddonClient.AddonV1alpha1() calls to AddonV1beta1() in beta test
Both alpha and v1beta1 tests cover:
- Addon status remains available while lease is updated
- Addon status changes to unavailable when lease stops updating
- Addon status changes to unknown when no lease exists
- Addon status changes to unknown when managed cluster lease stops updating
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: migrate addon_token_auth test to alpha/beta versions
- Rename addon_token_auth_test.go to addon_token_auth_alpha_test.go
- Update alpha Describe to include (v1alpha1)
- Create new addon_token_auth_test.go for v1beta1
- Update imports and client calls to use v1beta1 API
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>
* test: add v1beta1 integration tests for addon API
Add comprehensive v1beta1 test coverage for addon API following alpha/beta
split pattern. Tests for both v1alpha1 and v1beta1 run in parallel with API
conversion handled automatically by the test environment.
Migrated tests (11 test suites):
- addon_manager_install: split into _alpha + beta versions
- addon_configs: split into _alpha + beta versions
- addon_manager_upgrade: split into _alpha + beta versions
- agent_deploy: split into _alpha + beta versions
- token_infrastructure: split into _alpha + beta versions
- addon_manager_template: renamed to _alpha only (AddOnTemplate API removed in v1beta1)
Changes:
- suite_test.go: renamed client import to support both API versions
- assertion_test.go: added alpha/beta config specs and beta helper functions
v1beta1 API compatibility fixes:
- Removed deprecated ConfigReferent field from ConfigReference structs
- Removed InstallNamespace from ManagedClusterAddOnSpec
- Updated RegistrationConfig to use Type field with nested KubeClient/CustomSigner configs
- KubeClientDriver moved from top-level Status to RegistrationConfig.KubeClient.Driver
- SignerName implicit for kubeClient type, explicit in CustomSigner for customSigner type
All integration tests now have complete v1beta1 coverage. E2E tests will be
migrated separately.
Refs: https://github.com/open-cluster-management-io/ocm/issues/1517
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add ConfigReferent to v1beta1 ConfigSpecHash test expectations
The v1beta1 ConfigSpecHash structure includes an inlined ConfigReferent
field that specifies the config namespace and name. The controller
populates this field in status, so test expectations must include it.
Added ConfigReferent to all ConfigSpecHash assertions throughout the
addon_manager_upgrade_test.go progressive rollout scenarios:
- Initial config deployment (configDefaultName)
- After upgrade to test1 config
- Progressive rollout start (clusters 0-1 with configUpdateName)
- Progressive rollout continuation (clusters 2-4 with configDefaultName)
- After rollout completion (configUpdateName with test2)
- Mixed state during test3 rollout (DesiredConfig test3, LastAppliedConfig test2)
- Final state with test3 config
This fixes v1beta1 integration test failures where assertions expected
empty ConfigReferent but the controller correctly populated the fields.
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: fix PR review issues in v1beta1 integration tests
Address all issues identified in PR #1546 code review:
1. Split assertion helper functions into Alpha/Beta versions
- assertClusterManagementAddOnAnnotations -> Alpha/Beta versions
- assertClusterManagementAddOnConditions -> Alpha/Beta versions
- assertClusterManagementAddOnNoConditions -> Alpha/Beta versions
- assertManagedClusterAddOnConditions -> Alpha/Beta versions
Original functions were hardcoded to v1alpha1 API but called from both
v1alpha1 and v1beta1 tests. Beta versions now use v1beta1 API correctly.
Note: assertClusterManagementAddOnAnnotationsBeta is a no-op since
AddonLifecycleAnnotationKey was removed in v1beta1 (lifecycle is always
managed by addon-manager in v1beta1).
2. Fix inverted JSON comparison in agent_deploy tests
- Original bug: compared raw JSON bytes which failed due to whitespace
- Tests were passing incorrectly (inverted logic + byte mismatch = false positive)
- Fix: unmarshal both JSONs and compare objects semantically
- Applied to both agent_deploy_alpha_test.go and agent_deploy_test.go
3. Fix clusterNames accumulation across test cases
- Added clusterNames = nil reset in BeforeEach blocks
- Prevents cleanup failures from stale cluster references
- Applied to both addon_manager_upgrade_alpha_test.go and _test.go
4. Add bounds checking to assertion helpers
- Added len(actual.Status.InstallProgressions) checks before indexing
- Prevents panic during Eventually polling when status not yet populated
- Applied to all 4 condition assertion functions (Alpha/Beta variants)
- Returns retryable error instead of panicking, allowing Eventually to continue
All 27 integration tests now pass consistently.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: fix timing issue in ProgressDeadline assertion
Fix flaky test failure in addon rollout ProgressDeadline tests where the
timeout would fire ~1 second earlier than expected from the test's perspective.
Root cause: The test was capturing the start timestamp AFTER updating the
ManifestWork status to False. When the work status changes, the addon controller
sets the ManagedClusterAddOn Progressing condition with a LastTransitionTime.
The rollout SDK uses that LastTransitionTime (not when the CMA is patched with
ProgressDeadline config) as the base for timeout calculation.
This meant:
1. Line 365: Work status set to False → Controller sets Progressing=True with LastTransitionTime=T1
2. Line 382: Test captures start=T2 (where T2 = T1 + ~1s)
3. Line 383: CMA patched with ProgressDeadline="5s"
4. Controller calculates timeout as T1 + 5s = T2 + 4s (appears as 4s to test)
Fix: Capture start BEFORE the work status update, ensuring the test's timer
starts before the controller's timer, so timeouts always appear ≥5s from start.
Verified by code review:
- pkg/addon/controllers/addonprogressing/controller.go:213 sets Progressing condition
- pkg/addon/controllers/addonconfiguration/graph.go:88,91 copies LastTransitionTime
- vendor/.../sdk-go/.../rollout.go:487 uses LastTransitionTime for timeout calc
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 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>
Replace imagebuilder with docker/podman for building images, enabling
e2e tests to run on macOS. This change supports both Docker Desktop
and Podman on macOS (including Apple Silicon) and Linux.
Key changes:
- Add test/kind-images.mk for auto-detecting runtime and architecture
- Update test-e2e target to build and load images automatically
- Add SKIP_IMAGE_BUILD flag to skip image building when only tests change
- Add clean-e2e-env target for proper test environment cleanup
- Simplify GitHub Actions workflows (remove imagebuilder steps)
- Set default KUBECONFIG to ~/.kube/config
- Add comprehensive e2e testing documentation
The build system now automatically:
- Detects docker or podman
- Detects kind cluster architecture (amd64/arm64)
- Builds images with correct platform flag
- Handles different image loading methods for docker vs podman
Tested successfully on macOS with podman and Apple Silicon (79/83 tests passed).
Signed-off-by: Jian Qiu <jqiu@redhat.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>
## Problem
The addon management e2e test "Template type addon should be configured
by addon deployment config for proxy" fails intermittently with:
"Stop creating csr since there are too many csr created already on hub"
## Root Cause
Race condition between CSR deletion in AfterEach and API/cache consistency:
1. AfterEach deletes CSRs via API
2. Returns success immediately after Delete() calls succeed
3. Next test's BeforeEach starts before deletions fully propagate
4. CSR controller's indexer cache still shows deleted CSRs
5. haltAddonCSRCreation() checks cache, sees >=10 CSRs, halts creation
## Solution
Modified AfterEach CSR cleanup to verify deletions complete:
- After deleting CSRs, return error to force Eventually() to retry
- Only return success when List() confirms 0 CSRs remain
- Ensures API consistency before next test starts
- Added logging to show CSR cleanup count for debugging
This ensures the cache has time to sync and prevents accumulation
of phantom CSRs across ordered test runs.
Signed-off-by: Jian Zhu <jiazhu@redhat.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* Replace legacy envtest setup with sdk-go shared script
Replace the legacy ensure-kubebuilder-tools target that manually installed
setup-envtest and downloaded kubebuilder binaries with the new shared
envtest-setup target from sdk-go. This uses the centralized ensure-envtest.sh
script which automatically detects K8s version from go.mod and manages
binary downloads.
Signed-off-by: xuezhaojun <zxue@redhat.com>
* Fix Job condition rules integration test for K8s 1.34
K8s 1.34 enforces stricter Job status validation requiring startTime,
completionTime, and SuccessCriteriaMet condition for completed Jobs.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: xuezhaojun <zxue@redhat.com>
---------
Signed-off-by: xuezhaojun <zxue@redhat.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Modified e2e test suite to validate image tags and fail tests when
tag-based images don't match the expected tag, while skipping validation
for digest-based images (SHA format).
Changes:
- Added validateImageFormat() helper to check image format (tag vs digest)
- Images using digest format (@sha256:...) skip validation
- Images using tag format (:tag) are validated against expected tag
- Tests fail with Expect() if tag validation fails
- Validation applies to test image variables and ClusterManager specs
- Only validates ClusterManager CR specs, not deployments
- Removed validateKlusterletImageSpecs() to avoid validation before resource creation
Bug fix:
- Fixed CI failure where image validation ran before Klusterlet was created
- The validation now only checks test inputs (which are used to create Klusterlet)
- This ensures Klusterlet has correct images by design without redundant validation
This fixes the BeforeSuite error:
"image validation failed: [failed to get Klusterlet:
klusterlets.operator.open-cluster-management.io
"e2e-universal-klusterlet" not found]"
🤖 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>
This commit fixes two bugs in the ManifestWorkReplicaSet e2e tests:
1. Use freshly fetched mwrSet instead of stale mwReplicaSet when checking
status summary. This ensures we're validating against the latest state
rather than the initial object.
2. Return descriptive error messages instead of nil error when condition
checks fail. This improves test debugging by providing clear failure
reasons.
These fixes improve test reliability and error reporting.
🤖 Assisted by Claude Code
Signed-off-by: $(git config user.name) <$(git config user.email)>
Signed-off-by: zhujian <jiazhu@redhat.com>
* ✨ Add e2e test for token-based authentication with template addons
This test validates the token-based authentication feature for template
addons introduced in PR #1363. It tests the complete authentication
lifecycle including switching between token and CSR authentication modes.
Test Flow:
1. Enable token-based authentication for addons on klusterlet
2. Deploy template addon and verify it uses token auth
3. Validate token field exists in hub kubeconfig secret
4. Test addon functionality with token authentication
5. Switch back to CSR-based authentication
6. Verify hub kubeconfig now uses client certificates
7. Test addon functionality with CSR authentication
8. Cleanup all resources
Key Features:
- Comprehensive validation of both token and CSR authentication
- No manual CSR approval needed (auto-approved by system)
- Works independently of klusterlet registration driver (grpc/csr)
- Uses label "addon-token-auth" for selective test execution
🤖 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 addon token auth test to use BeforeAll/AfterAll hooks
Move klusterlet configuration save/restore logic from defer in test
function to BeforeAll/AfterAll hooks for better test structure and
isolation.
Changes:
- Save original klusterlet configuration in BeforeAll before any setup
- Configure token auth for klusterlet in BeforeAll
- Restore original configuration in AfterAll after cleanup
- Remove redundant Steps 9-12 (CSR auth switch back)
- Renumber remaining steps from 1-10
- Remove unused strings import
This ensures the klusterlet's original AddOnKubeClientRegistrationDriver
is preserved for other tests and provides clearer separation of test
setup/teardown from test logic.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: zhujian <jiazhu@redhat.com>
* ✅ Wait for registration agent rollout before proceeding in token auth test
Add explicit wait for registration agent deployment to fully rollout after
token authentication configuration is applied. This ensures all replicas are
updated and ready before proceeding with the test, preventing race conditions.
The wait validates:
- ObservedGeneration matches current generation
- All replicas are updated with new configuration
- All replicas are ready and available
🤖 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>
* ✨ Add deployment generation check to ensure token auth rollout
Capture the registration agent deployment generation before updating
the klusterlet configuration, then wait for it to increment after the
update. This ensures the test waits for the actual new deployment with
token auth configuration, not an old one with CSR-based auth.
Changes:
- Capture initial deployment generation before klusterlet update
- Calculate deployment name once based on Singleton vs Default mode
- Wait for deployment generation to increment after config change
- Verify deployment has fully rolled out with all pods updated and ready
This prevents race conditions where the test proceeds while old pods
with the previous CSR-based configuration are still running, which was
likely causing CI failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: zhujian <jiazhu@redhat.com>
* ✨ Add support for hosted mode in addon token auth test
This commit adds proper support for hosted mode deployment in the addon
token authentication e2e test. In hosted mode, the agent deployments run
on the hub cluster instead of the spoke cluster, and the agent namespace
is named after the klusterlet name rather than using a fixed namespace.
Key changes:
- Check for both InstallModeHosted and InstallModeSingletonHosted modes
- Use hub.KubeClient instead of spoke.KubeClient in hosted mode
- Use klusterlet.Name as agentNamespace in hosted mode
- Support InstallModeSingletonHosted for deployment naming
This ensures the test works correctly in all deployment modes:
Default, Singleton, Hosted, and SingletonHosted.
🤖 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>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.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>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 25s
Post / images (amd64, placement) (push) Failing after 47s
Post / images (amd64, registration) (push) Failing after 44s
Post / images (amd64, registration-operator) (push) Failing after 44s
Post / images (amd64, work) (push) Failing after 43s
Post / images (arm64, addon-manager) (push) Failing after 42s
Post / images (arm64, placement) (push) Failing after 41s
Post / images (arm64, registration) (push) Failing after 43s
Post / images (arm64, registration-operator) (push) Failing after 41s
Post / images (arm64, work) (push) Failing after 41s
Post / images (amd64, addon-manager) (push) Failing after 7m45s
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 38m55s
Close stale issues and PRs / stale (push) Successful in 50s
* sync clusterprofile based on managedclusterset and managedclustersetbinding
Co-authored-by: Claude <claude@anthropic.com>
Signed-off-by: Morven Cao <lcao@redhat.com>
* Refactor ClusterProfile controller into two separate controllers.
Signed-off-by: Morven Cao <lcao@redhat.com>
* address comments.
Signed-off-by: Morven Cao <lcao@redhat.com>
* fix lint issues.
Signed-off-by: Morven Cao <lcao@redhat.com>
* address comments.
Signed-off-by: Morven Cao <lcao@redhat.com>
* address comments.
Signed-off-by: Morven Cao <lcao@redhat.com>
---------
Signed-off-by: Morven Cao <lcao@redhat.com>
This commit adds validation to detect and reject duplicate manifests
in ManifestWork resources. A manifest is considered duplicate when
it has the same apiVersion, kind, namespace, and name as another
manifest in the same ManifestWork.
This prevents issues where duplicate manifests with different specs
can cause state inconsistency, as the Work Agent applies manifests
sequentially and later entries would overwrite earlier ones.
The validation returns a clear error message indicating the duplicate
manifest's index and the index of its first occurrence.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: xuezhaojun <zxue@redhat.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 13s
Post / images (amd64, addon-manager) (push) Failing after 48s
Post / images (amd64, placement) (push) Failing after 1m22s
Post / images (amd64, registration) (push) Failing after 42s
Post / images (amd64, work) (push) Failing after 41s
Post / images (arm64, addon-manager) (push) Failing after 42s
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, registration-operator) (push) Failing after 21m14s
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 39m11s
Close stale issues and PRs / stale (push) Successful in 50s
* Fix work rolebinding cleanup when hubAcceptsClient is set to false
Signed-off-by: Erico G. Rimoli <erico.rimoli@totvs.com.br>
* Adds error handling to the removeClusterRbac call within the controller synchronization function
Signed-off-by: Erico G. Rimoli <erico.rimoli@totvs.com.br>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 20s
Post / images (amd64, placement) (push) Failing after 45s
Post / images (amd64, registration) (push) Failing after 42s
Post / images (amd64, registration-operator) (push) Failing after 40s
Post / images (amd64, work) (push) Failing after 41s
Post / images (arm64, addon-manager) (push) Failing after 41s
Post / images (arm64, placement) (push) Failing after 40s
Post / images (arm64, registration) (push) Failing after 39s
Post / images (arm64, registration-operator) (push) Failing after 39s
Post / images (arm64, work) (push) Failing after 41s
Post / images (amd64, addon-manager) (push) Failing after 7m30s
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 9m44s
Update with success count
Remove status references
Add unit tests
Fix unit tests
Update unit tests
Test fix
Fix tests for lastTransitionTime
Fix integration tests
Signed-off-by: annelau <annelau@salesforce.com>
Co-authored-by: annelau <annelau@salesforce.com>
Post / images (amd64, addon-manager) (push) Failing after 43s
Post / images (amd64, placement) (push) Failing after 36s
Post / images (amd64, registration) (push) Failing after 36s
Post / images (amd64, registration-operator) (push) Failing after 36s
Post / images (amd64, work) (push) Failing after 38s
Post / images (arm64, placement) (push) Failing after 37s
Post / images (arm64, registration) (push) Failing after 37s
Post / images (arm64, registration-operator) (push) Failing after 38s
Post / images (arm64, work) (push) Failing after 38s
Post / images (arm64, addon-manager) (push) Failing after 14m20s
Scorecard supply-chain security / Scorecard analysis (push) Failing after 1m28s
Post / image manifest (addon-manager) (push) Has been cancelled
Post / image manifest (placement) (push) Has been cancelled
Post / image manifest (registration) (push) Has been cancelled
Post / image manifest (registration-operator) (push) Has been cancelled
Post / image manifest (work) (push) Has been cancelled
Post / trigger clusteradm e2e (push) Has been cancelled
Close stale issues and PRs / stale (push) Successful in 4s
Update code changes to only update observed generation without lastTransitionTime
Update with simple tests
Update with the latest PR changes
Add unit test changes
Add integration test generated by cursor
Fix unit tests
Signed-off-by: annelau <annelau@salesforce.com>
Co-authored-by: annelau <annelau@salesforce.com>
Post / images (amd64, addon-manager) (push) Failing after 46s
Post / images (amd64, placement) (push) Failing after 41s
Post / images (amd64, registration-operator) (push) Failing after 39s
Post / images (amd64, work) (push) Failing after 42s
Post / images (arm64, addon-manager) (push) Failing after 39s
Post / images (arm64, placement) (push) Failing after 39s
Post / images (arm64, registration) (push) Failing after 40s
Post / images (arm64, registration-operator) (push) Failing after 42s
Post / images (arm64, work) (push) Failing after 39s
Post / images (amd64, registration) (push) Failing after 7m46s
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 14m33s
Scorecard supply-chain security / Scorecard analysis (push) Failing after 1m25s
Close stale issues and PRs / stale (push) Successful in 46s
* Add addon conversion webhook for v1alpha1/v1beta1 API migration
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Qing Hao <qhao@redhat.com>
* Fix GroupVersion compatibility issues after API dependency update
This commit fixes compilation and test errors introduced by updating
the API dependency to use native conversion functions from PR #411.
Changes include:
1. Fix GroupVersion type mismatches across the codebase:
- Updated OwnerReference creation to use schema.GroupVersion
- Fixed webhook scheme registration to use proper GroupVersion type
- Applied fixes to addon, placement, migration, work, and registration controllers
2. Enhance addon conversion webhook:
- Use native API conversion functions from addon/v1beta1/conversion.go
- Fix InstallNamespace annotation key to match expected format
- Add custom logic to populate deprecated ConfigReferent field in ConfigReferences
- Properly preserve annotations during v1alpha1 <-> v1beta1 conversion
3. Remove duplicate conversion code:
- Deleted pkg/addon/webhook/conversion/ directory (~500 lines)
- Now using native conversion functions from the API repository
4. Patch vendored addon-framework:
- Fixed GroupVersion errors in agentdeploy utils
All unit tests pass successfully (97 packages, 0 failures).
Signed-off-by: Qing Hao <qhao@redhat.com>
---------
Signed-off-by: Qing Hao <qhao@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Skip garbage collection for ManifestWorks that have the
ManifestWorkReplicaSet controller label, as these should be
managed exclusively by the ManifestWorkReplicaSet controller.
Changes:
- Fix logic bug in controller to properly check for ReplicaSet label
- Add unit tests for label-based GC skip behavior
- Add integration test to verify GC skip for ReplicaSet-managed works
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: Jian Qiu <jqiu@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>