diff --git a/pkg/manager/hcp_proxy.go b/pkg/manager/hcp_proxy.go index e67f982b..bc08bd7a 100644 --- a/pkg/manager/hcp_proxy.go +++ b/pkg/manager/hcp_proxy.go @@ -539,9 +539,14 @@ func (p *hcpProxy) checkHubPermission( 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") } // API exists but the SA has no admin bindings — expected. Proceed to step 2. } else { diff --git a/pkg/manager/hcp_proxy_test.go b/pkg/manager/hcp_proxy_test.go index cab21cd2..7ce032b1 100644 --- a/pkg/manager/hcp_proxy_test.go +++ b/pkg/manager/hcp_proxy_test.go @@ -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) @@ -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 @@ -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) diff --git a/test/e2e/addon-manager-deployment.yaml b/test/e2e/addon-manager-deployment.yaml index f02d996f..cee61272 100644 --- a/test/e2e/addon-manager-deployment.yaml +++ b/test/e2e/addon-manager-deployment.yaml @@ -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