-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add HCP proxy CreateRequest to create HostedClusters from the hub #760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
openshift-merge-bot
merged 2 commits into
stolostron:main
from
yiraeChristineKim:ACM-37268
Jul 22, 2026
+4,995
−85
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| --- | ||
| description: Sync hub-side HCP proxy resources to backplane-operator. Apply when editing pkg/manager/hcp_proxy.go or related hub manager wiring. | ||
| globs: | ||
| - pkg/manager/hcp_proxy.go | ||
| - pkg/manager/manager.go | ||
| --- | ||
|
|
||
| # backplane-operator Sync Rule | ||
|
|
||
| Hub-side HCP proxy resources (Service, APIService, ClusterRole/Binding, Deployment | ||
| ports, manager ClusterRole rules) are provisioned by **backplane-operator**, not | ||
| applied at runtime by this operator: | ||
|
|
||
| `pkg/templates/charts/toggle/hypershift/templates/` | ||
| (in the `stolostron/backplane-operator` repository) | ||
|
|
||
| E2E copies of the manifests live in `test/e2e/addon-manager-deployment.yaml` for kind only. | ||
|
|
||
| ## Required syncs | ||
|
|
||
| ### New hub-facing ports | ||
| → Add `containerPort` entry to `hypershift-addon-manager-deployment.yaml` | ||
|
|
||
| ```yaml | ||
| ports: | ||
| - containerPort: 9443 | ||
| name: hcp-proxy | ||
| protocol: TCP | ||
| ``` | ||
|
|
||
| ### HCP proxy Service / APIService / RBAC | ||
| → Add or update templates in backplane-operator (Service targeting port 9443, | ||
| APIService `v1alpha1.hcp.ocm.io`, ClusterRole/Binding for the manager SA). | ||
|
|
||
| ### New hub manager RBAC rules | ||
| → Add rules to `hypershift-addon-manager_clusterrole.yaml` | ||
|
|
||
| Currently required for the HCP proxy: | ||
| ```yaml | ||
| - apiGroups: ["clusterview.open-cluster-management.io"] | ||
| resources: ["userpermissions"] | ||
| verbs: ["get"] | ||
| - apiGroups: ["cluster.open-cluster-management.io"] | ||
| resources: ["managedclusters"] | ||
| verbs: ["get", "list", "watch"] | ||
| - apiGroups: ["operator.open-cluster-management.io"] | ||
| resources: ["multiclusterhubs"] | ||
| verbs: ["get", "list", "watch"] | ||
| - apiGroups: ["config.openshift.io"] | ||
| resources: ["apiservers"] | ||
| verbs: ["get", "list", "watch"] | ||
| - apiGroups: [""] | ||
| resources: ["users", "groups", "serviceaccounts"] | ||
| verbs: ["impersonate"] | ||
| - apiGroups: ["authentication.k8s.io"] | ||
| resources: ["userextras"] | ||
| verbs: ["impersonate"] | ||
| ``` | ||
|
|
||
| The `config.openshift.io/apiservers` rule is required so SecurityProfileWatcher | ||
| and `FetchAPIServerTLSProfile` can read the cluster TLS profile. | ||
|
|
||
| ### New hub ServiceAccount | ||
| → Add entry to `hypershift-addon-manager-serviceaccount.yaml` | ||
|
|
||
| ## Why | ||
| The hub manager pod (Deployment, ClusterRole, ServiceAccount, Service, APIService) | ||
| is defined in `backplane-operator`, not applied by `StartHCPProxy` in this repo. | ||
|
|
||
| Upstream PR reference: https://github.com/stolostron/backplane-operator/tree/main/pkg/templates/charts/toggle/hypershift/templates |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| --- | ||
| description: > | ||
| OpenShift central TLS profile compliance (ACM-26882, release blocker OCP 4.23 / ACM 5.0). | ||
| Apply whenever writing or reviewing any code that opens a TLS server, creates an outbound | ||
| HTTP/HTTPS client, generates certificates, or touches TLS configuration. | ||
| globs: | ||
| - "**/*.go" | ||
| alwaysApply: true | ||
| --- | ||
|
|
||
| # TLS Compliance — Mandatory Pattern | ||
|
|
||
| This repo is covered by **ACM-26882 "Central TLS Profile consistency"**, which is a release | ||
| blocker for OCP 4.23 / ACM 5.0. Every piece of new or modified TLS code **MUST** follow the | ||
| four rules below. Violations will block merges. | ||
|
|
||
| ## The four rules | ||
|
|
||
| ### 1. Fetch the profile once at manager startup | ||
|
|
||
| In `manager.go` (or equivalent entry-point), call **before** starting any server or client: | ||
|
|
||
| ```go | ||
| import tlspkg "github.com/openshift/controller-runtime-common/pkg/tls" | ||
|
|
||
| profileSpec, err := tlspkg.FetchAPIServerTLSProfile(ctx, hubClient) | ||
| if err != nil { | ||
| // kind / non-OpenShift clusters lack the resource — use Intermediate fallback | ||
| log.Error(err, "failed to fetch APIServer TLS profile, using Intermediate defaults") | ||
| profileSpec, _ = tlspkg.GetTLSProfileSpec(nil) | ||
| } | ||
| ``` | ||
|
|
||
| Pass `profileSpec` explicitly to every server and client constructor — **never re-fetch it | ||
| inside a loop or per-request**. | ||
|
|
||
| ### 2. TLS server — apply profile via `tlspkg.NewTLSConfigFromProfile` | ||
|
|
||
| ```go | ||
| // NewTLSConfigFromProfile returns (applyFn, unsupportedCipherNames) — not an error. | ||
| tlsCfgFn, unsupported := tlspkg.NewTLSConfigFromProfile(profileSpec) | ||
| if len(unsupported) > 0 { | ||
| log.Info("TLS profile contains unsupported ciphers, they will be ignored", "ciphers", unsupported) | ||
| } | ||
| cert, err := generateSelfSignedCert(operatorNS) // see rule 4 | ||
| if err != nil { | ||
| return fmt.Errorf("generate serving cert: %w", err) | ||
| } | ||
| tlsCfg := &tls.Config{Certificates: []tls.Certificate{cert}} | ||
| tlsCfgFn(tlsCfg) // sets MinVersion + CipherSuites from central profile | ||
|
|
||
| srv := &http.Server{TLSConfig: tlsCfg, ...} | ||
| ``` | ||
|
|
||
| **Never** set `tls.Config.MinVersion`, `tls.Config.CipherSuites`, or `tls.Config.CurvePreferences` | ||
| by hand. Those values must come exclusively from the profile function. | ||
|
|
||
| ### 3. Outbound HTTP client — use `buildHTTPClient` | ||
|
|
||
| Every outbound HTTPS call must go through the shared helper that applies the profile to the | ||
| transport: | ||
|
|
||
| ```go | ||
| // buildHTTPClient creates an http.Client whose TLS transport honours the cluster | ||
| // TLS profile (MinVersion, CipherSuites) and carries the hub bearer token. | ||
| func (p *hcpProxy) buildHTTPClient(timeout time.Duration) (*http.Client, error) { | ||
| tlsCfg, err := rest.TLSConfigFor(p.hubConfig) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("TLS config from rest.Config: %w", err) | ||
| } | ||
| tlsConfigFn, _ := tlspkg.NewTLSConfigFromProfile(p.profileSpec) | ||
| tlsConfigFn(tlsCfg) // overlay MinVersion + CipherSuites | ||
|
|
||
| base := &http.Transport{TLSClientConfig: tlsCfg, ...} | ||
| wrapped, err := rest.HTTPWrappersForConfig(p.hubConfig, base) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("HTTP auth wrappers: %w", err) | ||
| } | ||
| return &http.Client{Transport: wrapped, Timeout: timeout}, nil | ||
| } | ||
| ``` | ||
|
|
||
| Do **not** create raw `http.Client` or `http.Transport` instances with a bare `tls.Config{}` — | ||
| always call `buildHTTPClient` (or an equivalent that applies `tlspkg.NewTLSConfigFromProfile`). | ||
|
|
||
| ### 4. Certificates — prefer service-ca, fall back to `libgocrypto` | ||
|
|
||
| **On OpenShift** the preferred source is the cluster Service CA. Annotate the Service: | ||
|
|
||
| ```yaml | ||
| metadata: | ||
| annotations: | ||
| service.beta.openshift.io/serving-cert-secret-name: <name>-tls | ||
| ``` | ||
|
|
||
| Mount the generated Secret with `optional: true` and use `loadOrGenerateCert` which: | ||
| 1. Loads `tls.crt` / `tls.key` from the mount path when present (OpenShift — trusted, auto-rotated) | ||
| 2. Falls back to `generateSelfSignedCert` when the path is absent (kind / vanilla k8s) | ||
|
|
||
| **On non-OpenShift clusters (kind, e2e)** the annotation is ignored and the fallback | ||
| in-process path activates automatically via `loadOrGenerateCert`. | ||
|
|
||
| For the fallback, use **`github.com/openshift/library-go/pkg/crypto`**: | ||
|
|
||
| ```go | ||
| import ( | ||
| libgocrypto "github.com/openshift/library-go/pkg/crypto" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
| ) | ||
|
|
||
| func generateSelfSignedCert(operatorNS string) (tls.Certificate, error) { | ||
| const certLifetime = 2 * 365 * 24 * time.Hour // ≤ 7200-day library-go limit | ||
|
|
||
| caConfig, err := libgocrypto.MakeSelfSignedCAConfigForDuration( | ||
| serviceName+"-ca", certLifetime) | ||
| if err != nil { | ||
| return tls.Certificate{}, fmt.Errorf("create CA: %w", err) | ||
| } | ||
| ca := &libgocrypto.CA{ | ||
| Config: caConfig, | ||
| SerialGenerator: &libgocrypto.RandomSerialGenerator{}, | ||
| } | ||
| hostnames := sets.New[string]("localhost", "127.0.0.1", | ||
| serviceName, serviceName+"."+operatorNS+".svc", | ||
| serviceName+"."+operatorNS+".svc.cluster.local") | ||
| serverConfig, err := ca.MakeServerCert(hostnames, certLifetime) | ||
| if err != nil { | ||
| return tls.Certificate{}, fmt.Errorf("create server cert: %w", err) | ||
| } | ||
| certPEM, keyPEM, err := serverConfig.GetPEMBytes() | ||
| if err != nil { | ||
| return tls.Certificate{}, fmt.Errorf("encode server cert PEM: %w", err) | ||
| } | ||
| return tls.X509KeyPair(certPEM, keyPEM) | ||
| } | ||
| ``` | ||
|
|
||
| **Never** use `crypto/ecdsa`, `crypto/x509.CreateCertificate`, `crypto/rand`, `math/big`, or | ||
| `encoding/pem` directly for certificate generation — those bypass OpenShift's key-type and | ||
| serial-number conventions. | ||
|
|
||
| ### 5. Runtime watch — register `SecurityProfileWatcher` | ||
|
|
||
| Any manager that embeds a TLS server must restart cleanly when the cluster profile changes: | ||
|
|
||
| ```go | ||
| watcher := &tlspkg.SecurityProfileWatcher{ | ||
| Client: hubClient, | ||
| InitialTLSProfileSpec: profileSpec, | ||
| OnProfileChange: func(_ context.Context, old, new configv1.TLSProfileSpec) { | ||
| log.Info("TLS profile changed, restarting", "old", old.MinTLSVersion, "new", new.MinTLSVersion) | ||
| cancelManager() // triggers graceful pod restart | ||
| }, | ||
| } | ||
| if err := watcher.SetupWithManager(mgr); err != nil { | ||
| return fmt.Errorf("setup TLS watcher: %w", err) | ||
| } | ||
| ``` | ||
|
|
||
| ## RBAC required | ||
|
|
||
| Whenever any of the above is used, the manager's ClusterRole must include: | ||
|
|
||
| ```yaml | ||
| - apiGroups: ["config.openshift.io"] | ||
| resources: ["apiservers"] | ||
| verbs: ["get", "list", "watch"] | ||
| ``` | ||
|
|
||
| Add it to the `backplane-operator` hypershift-addon-manager ClusterRole | ||
| (see `.cursor/rules/backplane-operator-sync.mdc`). For kind e2e, also keep | ||
| `test/e2e/addon-manager-deployment.yaml` in sync. | ||
|
|
||
| ## Forbidden patterns (will fail code review) | ||
|
|
||
| | Forbidden | Use instead | | ||
| |---|---| | ||
| | `tls.Config{MinVersion: tls.VersionTLS12}` | `tlspkg.NewTLSConfigFromProfile(profileSpec)` | | ||
| | `tls.Config{MinVersion: tls.VersionTLS13}` | same | | ||
| | `tls.Config{CipherSuites: [...]}` (hardcoded) | same | | ||
| | `http.Client{Transport: &http.Transport{TLSClientConfig: ...}}` without profile | `buildHTTPClient(...)` | | ||
| | `x509.CreateCertificate(rand.Reader, ...)` | `libgocrypto.MakeSelfSignedCAConfigForDuration` | | ||
| | `ecdsa.GenerateKey(elliptic.P256(), ...)` for serving cert | `libgocrypto.MakeServerCert` | | ||
|
|
||
| ## Reference | ||
|
|
||
| - Epic: [ACM-26882](https://redhat.atlassian.net/browse/ACM-26882) — Central TLS Profile consistency | ||
| - HyperShift addon task: [ACM-30178](https://redhat.atlassian.net/browse/ACM-30178) — verified ACM 5.0.0-112 | ||
| - Technical guide: https://docs.google.com/document/d/1cMc9E8psHfnoK06ntR8kHSWB8d3rMtmldhnmM4nImjs | ||
| - Slack: [#forum-ocp-tls-strict-obedience](https://redhat.enterprise.slack.com/archives/C098FU5MRAB) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| .git | ||
| .github | ||
| .cursor | ||
| .work | ||
| docs | ||
| examples | ||
| quickstart | ||
| *.md | ||
| cover.out | ||
| e2e.test | ||
| bin/* | ||
| !bin/hypershift-addon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| name: E2E – HCP Proxy | ||
|
|
||
| on: | ||
| push: | ||
| branches: ["main", "backplane-5.*"] | ||
| paths: | ||
| - "pkg/manager/hcp_proxy.go" | ||
| - "pkg/manager/manager.go" | ||
| - "test/e2e/**" | ||
| - "Dockerfile.e2e" | ||
| - "Makefile" | ||
| - "hack/install_ocm.sh" | ||
| - ".github/workflows/e2e-hcp-proxy.yaml" | ||
| pull_request: | ||
| branches: ["main", "backplane-5.*"] | ||
| paths: | ||
| - "pkg/manager/hcp_proxy.go" | ||
| - "pkg/manager/manager.go" | ||
| - "test/e2e/**" | ||
| - "Dockerfile.e2e" | ||
| - "Makefile" | ||
| - "hack/install_ocm.sh" | ||
| - ".github/workflows/e2e-hcp-proxy.yaml" | ||
| workflow_dispatch: {} | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| env: | ||
| GO_VERSION: "1.26" | ||
| KIND_CLUSTER_NAME: hcp-proxy-e2e | ||
|
|
||
| jobs: | ||
| e2e-hcp-proxy: | ||
| name: HCP Proxy E2E | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 40 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 | ||
| with: | ||
| go-version: ${{ env.GO_VERSION }} | ||
| cache: true | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| cache-dependency-path: go.sum | ||
|
|
||
| # Warm the module cache before kind/OCM so the parallel image build hits cache. | ||
| - name: Download Go modules | ||
| run: go mod download | ||
|
|
||
| - name: Set up cluster and deploy addon manager | ||
| run: make e2e-hcp-proxy-setup KIND_CLUSTER_NAME=${{ env.KIND_CLUSTER_NAME }} | ||
|
|
||
| - name: Run HCP Proxy e2e tests | ||
| run: make test-e2e-hcp-proxy KIND_CLUSTER_NAME=${{ env.KIND_CLUSTER_NAME }} | ||
|
|
||
| - name: Dump addon manager logs on failure | ||
| if: failure() | ||
| run: kubectl logs -n multicluster-engine -l app=hypershift-addon-manager --tail=200 || true | ||
|
|
||
| - name: Delete kind cluster | ||
| if: always() | ||
| run: make kind-delete KIND_CLUSTER_NAME=${{ env.KIND_CLUSTER_NAME }} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| { | ||
| "version": "0.2.0", | ||
| "configurations": [ | ||
| { | ||
| "name": "Hub Manager", | ||
| "type": "go", | ||
| "request": "launch", | ||
| "mode": "auto", | ||
| "program": "${workspaceFolder}/cmd/main.go", | ||
| "args": [ | ||
| "manager", | ||
| "--disable-leader-election", | ||
| "--kubeconfig", "${env:HOME}/.kube/config", | ||
| "--disable-tls-watcher" | ||
| ], | ||
| "env": { | ||
| }, | ||
| "cwd": "${workspaceFolder}", | ||
| "console": "internalConsole" | ||
| }, | ||
| { | ||
| "name": "Spoke Agent", | ||
| "type": "go", | ||
| "request": "launch", | ||
| "mode": "auto", | ||
| "program": "${workspaceFolder}/cmd/main.go", | ||
| "args": [ | ||
| "agent", | ||
| "--hub-kubeconfig", "${env:HOME}/.kube/config", | ||
| "--cluster-name", "local-cluster", | ||
| "--addon-namespace", "hypershift", | ||
| "--hypershfit-operator-image", "quay.io/hypershift/hypershift-operator:latest" | ||
| ], | ||
| "cwd": "${workspaceFolder}", | ||
| "console": "internalConsole" | ||
| }, | ||
| { | ||
| "name": "Cleanup", | ||
| "type": "go", | ||
| "request": "launch", | ||
| "mode": "auto", | ||
| "program": "${workspaceFolder}/cmd/main.go", | ||
| "args": [ | ||
| "cleanup", | ||
| "--hub-kubeconfig", "${env:HOME}/.kube/config", | ||
| "--cluster-name", "local-cluster", | ||
| "--addon-namespace", "hypershift" | ||
| ], | ||
| "cwd": "${workspaceFolder}", | ||
| "console": "internalConsole" | ||
| } | ||
| ] | ||
| } | ||
|
yiraeChristineKim marked this conversation as resolved.
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.