* 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>
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>
Dale HaiducekandGitHub
shivansh sinhaandGitHub
Zhiwei YinandGitHub
kahirokunnandGitHub
Jian ZhuandGitHub
Jian QiuandGitHub
Mike NgandGitHub
Zhiwei YinandGitHub
Andy AndersonandGitHub