Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions pkg/manager/hcp_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,14 @@
if _, probeErr := p.hubDynClient.Resource(gvr).Get(ctx, "managedcluster:admin", metav1.GetOptions{}); probeErr != nil {
if apierrors.IsNotFound(probeErr) {
if strings.Contains(probeErr.Error(), "the server could not find the requested resource") {
// API group is not registered (kind / non-ACM hub) — skip non-fatally.
p.log.Info("clusterview API not installed, skipping hub permission check")
return nil
// API group is not registered — only skip in E2E/kind environments.
// On production clusters this could indicate a partial MCE install failure;
// skipping would be a security risk.
if os.Getenv("SKIP_HUB_PERMISSION_CHECK") == "true" {
p.log.Info("clusterview API not installed, skipping hub permission check (SKIP_HUB_PERMISSION_CHECK=true)")
return nil
}
return fmt.Errorf("UserPermission is required in production. Please ensure the cluster has UserPermission configured")

Check warning on line 549 in pkg/manager/hcp_proxy.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Split this 122 characters long line (which is greater than 120 authorized).

See more on https://sonarcloud.io/project/issues?id=open-cluster-management_hypershift-addon-operator&issues=AZ-lIYo1Vdc5U94j_ROX&open=AZ-lIYo1Vdc5U94j_ROX&pullRequest=765
}
// API exists but the SA has no admin bindings — expected. Proceed to step 2.
} else {
Expand Down
29 changes: 25 additions & 4 deletions pkg/manager/hcp_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,11 @@ func Test_checkHubPermission_WhenViewOnlyUser_ItShouldReturnError(t *testing.T)
assert.Contains(t, err.Error(), "does not have admin access")
}

func Test_checkHubPermission_WhenClusterviewAPIAbsent_ItShouldSkipAndAllow(t *testing.T) {
// Simulates a kind/non-ACM hub: every request returns 404 with the
// "server could not find the requested resource" message, meaning the API group
// is not installed at all — the check is skipped non-fatally.
func Test_checkHubPermission_WhenClusterviewAPIAbsent_WithEnvVar_ItShouldSkipAndAllow(t *testing.T) {
// Simulates a kind/non-ACM hub with SKIP_HUB_PERMISSION_CHECK=true:
// the API group is not installed and the env var allows skipping.
t.Setenv("SKIP_HUB_PERMISSION_CHECK", "true")

hubSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set(headerContentType, contentTypeJSON)
w.WriteHeader(http.StatusNotFound)
Expand All @@ -495,6 +496,25 @@ func Test_checkHubPermission_WhenClusterviewAPIAbsent_ItShouldSkipAndAllow(t *te
assert.NoError(t, err)
}

func Test_checkHubPermission_WhenClusterviewAPIAbsent_WithoutEnvVar_ItShouldFailClosed(t *testing.T) {
// Simulates a partial MCE install failure where the API is absent but
// SKIP_HUB_PERMISSION_CHECK is not set — should fail closed for security.
t.Setenv("SKIP_HUB_PERMISSION_CHECK", "")

hubSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set(headerContentType, contentTypeJSON)
w.WriteHeader(http.StatusNotFound)
_, _ = io.WriteString(w, `{"kind":"Status","apiVersion":"v1","reason":"NotFound",`+
`"message":"the server could not find the requested resource"}`)
}))
defer hubSrv.Close()

p := newTestProxyWithHubServer(t, hubSrv.URL)
err := p.checkHubPermission(context.Background(), "anyuser", nil, "spoke-1")
assert.Error(t, err)
assert.Contains(t, err.Error(), "UserPermission is required in production")
}

func Test_checkHubPermission_WhenProbeReturnsResourceNotFound_ItShouldProceedToStep2(t *testing.T) {
// Simulates a real ACM hub where the clusterview API exists but the operator SA
// has no managedcluster:admin bindings. The probe (step 1) returns a proper resource-level
Expand Down Expand Up @@ -1600,6 +1620,7 @@ func availableManagedCluster(name string) *clusterv1.ManagedCluster {
// checkHubPermission (used by handleRoute) skips non-fatally in unit tests.
func newTestProxyWithSpokeURL(t *testing.T, spokeServerURL string, objs ...runtime.Object) *hcpProxy {
t.Helper()
t.Setenv("SKIP_HUB_PERMISSION_CHECK", "true")
p := newTestProxy(t, objs...)
hubSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set(headerContentType, contentTypeJSON)
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/addon-manager-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ spec:
# Skip TLS verify for the self-signed user-server serving cert.
- name: CLUSTER_PROXY_INSECURE
value: "true"
# kind has no clusterview API — skip hub permission check.
- name: SKIP_HUB_PERMISSION_CHECK
value: "true"
volumeMounts:
- mountPath: /etc/hcp-proxy/tls
name: hcp-proxy-tls
Expand Down
Loading