Skip to content

CNTRLPLANE-2677: Add HCPEtcdBackup CRD API and feature gate - #7898

Merged
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
jparrill:CNTRLPLANE-2677
Mar 18, 2026
Merged

CNTRLPLANE-2677: Add HCPEtcdBackup CRD API and feature gate#7898
openshift-merge-bot[bot] merged 2 commits into
openshift:mainfrom
jparrill:CNTRLPLANE-2677

Conversation

@jparrill

@jparrill jparrill commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Register the HCPEtcdBackup feature gate (disabled in Default, enabled in TechPreviewNoUpgrade)
  • Define the HCPEtcdBackup CRD API types (HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus, S3/AzureBlob storage configs, encryption metadata)
  • Add HCPEtcdBackupConfig with platform-specific blocks (AWS/Azure) to ManagedEtcdSpec for HC→HCP propagation of backup encryption config
  • Generate deepcopy methods and featuregated CRD manifests via make hypershift-api

Validation

  • Spec immutability: CEL self == oldSelf on the entire HCPEtcdBackupSpec (one-shot backup request)
  • Union discriminator enforcement: CEL rules ensure storageType: S3 requires s3 block and storageType: AzureBlob requires azureBlob block
  • HCPEtcdBackupConfig mutual exclusion: CEL ensures exactly one of aws or azure is specified
  • Regexp validation: kmsKeyARN requires ^arn: pattern, encryptionKeyURL requires ^https:// pattern (following precedents in aws.go)
  • Field-level immutability: kmsKeyARN and encryptionKeyURL are individually immutable via self == oldSelf

Conventions

  • Pointers used only where nil vs zero has semantic difference (union members with has() CEL, optional feature-gated struct)
  • EncryptionMetadata in status is a value type (no nil vs zero semantic difference)
  • All fields have omitempty (strings) or omitzero (struct pointers) per OpenShift API conventions
  • +required markers only (no +kubebuilder:validation:Required)

This is the first user story in the HCPEtcdBackup epic (CNTRLPLANE-2676). It defines the CRD types, feature gate, and propagation path. No controller logic is included.

JIRA

Test plan

  • go test ./hypershift-operator/featuregate/... — feature gate tests pass (HCPEtcdBackup disabled in Default, enabled in TechPreview)
  • make hypershift-api — generates deepcopy and CRD manifests without errors
  • go build ./... — full build compiles successfully
  • go test ./cmd/install/... — CRD install tests pass
  • Verified ManagedEtcdSpec.DeepCopyInto includes the new Backup field
  • Verified CRD manifests generated in cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/
  • make lint-fix && make verify — linter and verify checks pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added HCPEtcdBackup resource for managing etcd backups with dual storage backend support (S3 and Azure Blob).
    • Introduced encryption configuration for backup artifacts with KMS support.
    • Added backup configuration to hosted cluster managed etcd specifications.
    • Backup status tracking with conditions and snapshot URL reporting.

@openshift-ci-robot

Copy link
Copy Markdown

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 9, 2026
@openshift-ci-robot

openshift-ci-robot commented Mar 9, 2026

Copy link
Copy Markdown

@jparrill: This pull request references CNTRLPLANE-2677 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

  • Register the HCPEtcdBackup feature gate (disabled in Default, enabled in TechPreviewNoUpgrade)
  • Define the HCPEtcdBackup CRD API types (HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus, S3/AzureBlob storage configs, encryption metadata)
  • Add HCPEtcdBackupConfig to ManagedEtcdSpec for HC→HCP propagation of backup configuration (leveraging existing DeepCopy() path)
  • Generate deepcopy methods and featuregated CRD manifests via make hypershift-api

This is the first user story in the HCPEtcdBackup epic (CNTRLPLANE-2676). It defines the CRD types, feature gate, and propagation path. No controller logic is included.

JIRA

Test plan

  • go test ./hypershift-operator/featuregate/... — feature gate tests pass (HCPEtcdBackup disabled in Default, enabled in TechPreview)
  • make hypershift-api — generates deepcopy and CRD manifests without errors
  • go build ./... — full build compiles successfully
  • go test ./cmd/install/... — CRD install tests pass
  • Verified ManagedEtcdSpec.DeepCopyInto includes the new Backup field
  • Verified CRD manifests generated in cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 42d48b3d-773e-4c7b-9f76-d1ee22c5e719

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new HCPEtcdBackup API/CRD and feature gate, extends HostedCluster/HostedControlPlane backup fields, and generates client/applyconfiguration/informer/lister/fake code, CRD manifests, operator feature-gate registration and tests, plus docs to support feature-gated etcd backups to S3 or Azure Blob with encryption and immutability rules.

Changes

Cohort / File(s) Summary
API types & registration
api/hypershift/v1beta1/etcdbackup_types.go
New HCPEtcdBackup types, enums (storage, KMS state), status/encryption metadata, constants, Scheme registration, kubebuilder/OpenShift markers and CRD annotations.
Feature gate manifests
api/hypershift/v1beta1/featuregates/*
Adds HCPEtcdBackup to enabled/disabled lists across Default/TechPreviewNoUpgrade and SelfManagedHA profiles.
HostedCluster / HostedControlPlane CRDs
api/hypershift/v1beta1/hostedcluster_types.go, cmd/install/assets/.../hostedclusters-*.crd.yaml, .../hostedcontrolplanes-*.crd.yaml
Introduces spec.etcd.managed.backup / ManagedEtcdSpec.Backup with AWS kmsKeyARN and Azure encryptionKeyURL, immutability, mutual-exclusion and at-least-one validations.
Generated HCPEtcdBackup CRD manifests
api/.../zz_generated.featuregated-crd-manifests/.../HCPEtcdBackup.yaml, cmd/install/assets/.../hcpetcdbackups-*.crd.yaml
Adds feature-gated HCPEtcdBackup CRD YAMLs with OpenAPI v3 schema, validations (including immutable fields), printer columns, and status subresource.
Operator feature gate code & tests
hypershift-operator/featuregate/feature.go, .../feature_test.go
Registers HCPEtcdBackup feature constant, creates feature instance and updates tests to include the new gate across feature sets.
Client applyconfigs & utils
client/applyconfiguration/hypershift/v1beta1/*, client/applyconfiguration/utils.go
Generated apply-configuration types and fluent builders for HCPEtcdBackup and subtypes; ForKind mapping entries updated; ManagedEtcdSpecApplyConfiguration gains Backup field and WithBackup builder.
Typed client, fake & expansions
client/clientset/.../hcpetcdbackup.go, client/.../fake/*, client/.../generated_expansion.go, client/.../fake/fake_hypershift_client.go
Typed HCPEtcdBackup client (CRUD/Apply/Status), expansion interfaces, fake client implementation and clientset getter added.
Informers & Listers
client/informers/.../hcpetcdbackup.go, client/informers/.../interface.go, client/listers/.../hcpetcdbackup.go, client/listers/.../expansion_generated.go, client/informers/externalversions/generic.go
Informer registration, informer implementation, lister and expansion placeholders for HCPEtcdBackup added; ForResource mapping extended.
Installer manifests
cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/*hcpetcdbackups*.crd.yaml
Installer CRD manifests added for HCPEtcdBackup (TechPreview/Custom variants) with schema and feature-gate annotations.
Docs & aggregated reference
docs/content/reference/*, docs/content/reference/aggregated-docs.md
Documentation and API reference updated to include HCPEtcdBackup types, fields, HostedCluster propagation notes and examples.
Small glue changes
client/applyconfiguration/managedetcdspec.go, client/informers/externalversions/generic.go
Applyconfig ManagedEtcdSpec.Backup field and WithBackup builder; informer ForResource entry added.

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant APIServer as API Server
  participant Controller as HCPEtcdBackup Controller
  participant Storage as S3/Azure
  participant KMS as KMS

  User->>APIServer: Create HCPEtcdBackup (spec: storageType, backend config, encryption)
  APIServer->>Controller: Notify controller of new HCPEtcdBackup
  Controller->>APIServer: Read secrets / HostedCluster backup config
  Controller->>KMS: (optional) Prepare or validate encryption key
  Controller->>Storage: Upload etcd snapshot with encryption
  Storage-->>Controller: Return snapshot URL / status
  Controller->>APIServer: Update HCPEtcdBackup.status (conditions, snapshotURL, encryptionMetadata)
  APIServer-->>User: status reflects BackupCompleted / BackupFailed
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ❓ Inconclusive PR appears to add standard Go unit tests for feature gate validation rather than Ginkgo BDD-style tests, making Ginkgo-specific quality requirements potentially inapplicable. Confirm whether PR introduces Ginkgo-style tests (Describe/It/BeforeEach/AfterEach/Eventually/Consistently) or only standard Go unit tests for feature gate configuration.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'CNTRLPLANE-2677: Add HCPEtcdBackup CRD API and feature gate' clearly and concisely summarizes the main change: introducing a new HCPEtcdBackup CRD resource with its API types and feature gate registration.
Stable And Deterministic Test Names ✅ Passed All test names added in this PR follow stable and deterministic naming conventions with no dynamic values.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
openshift-ci Bot requested review from csrwng and devguyio March 9, 2026 21:52
@openshift-ci

openshift-ci Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jparrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Mar 9, 2026
@jparrill
jparrill marked this pull request as draft March 9, 2026 21:52
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2026
@jparrill

jparrill commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

/test unit

@jparrill

jparrill commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

/test verify

@jparrill

jparrill commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-aws

@openshift-ci-robot

openshift-ci-robot commented Mar 9, 2026

Copy link
Copy Markdown

@jparrill: This pull request references CNTRLPLANE-2677 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

  • Register the HCPEtcdBackup feature gate (disabled in Default, enabled in TechPreviewNoUpgrade)
  • Define the HCPEtcdBackup CRD API types (HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus, S3/AzureBlob storage configs, encryption metadata)
  • Add HCPEtcdBackupConfig to ManagedEtcdSpec for HC→HCP propagation of backup configuration (leveraging existing DeepCopy() path)
  • Generate deepcopy methods and featuregated CRD manifests via make hypershift-api

This is the first user story in the HCPEtcdBackup epic (CNTRLPLANE-2676). It defines the CRD types, feature gate, and propagation path. No controller logic is included.

JIRA

Test plan

  • go test ./hypershift-operator/featuregate/... — feature gate tests pass (HCPEtcdBackup disabled in Default, enabled in TechPreview)
  • make hypershift-api — generates deepcopy and CRD manifests without errors
  • go build ./... — full build compiles successfully
  • go test ./cmd/install/... — CRD install tests pass
  • Verified ManagedEtcdSpec.DeepCopyInto includes the new Backup field
  • Verified CRD manifests generated in cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
  • Added etcd backup capability with support for S3 and Azure Blob storage backends.
  • Introduced encryption configuration for backup artifacts using AWS KMS and Azure Key Vault.
  • Feature available in tech preview mode (feature-gated).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
api/hypershift/v1beta1/etcdbackup_types.go (1)

51-68: Add CEL validation to enforce union discriminator constraints.

The union discriminator markers (+unionDiscriminator, +unionMember) provide documentation but don't enforce runtime validation. Consider adding +kubebuilder:validation:XValidation rules to ensure:

  1. The correct storage config is provided for the selected storageType.
  2. Only one storage config is provided at a time.
Suggested CEL validation markers
 // HCPEtcdBackupSpec defines the desired state of HCPEtcdBackup.
+// +kubebuilder:validation:XValidation:rule="self.storageType != 'S3' || has(self.s3)",message="s3 configuration is required when storageType is S3"
+// +kubebuilder:validation:XValidation:rule="self.storageType != 'AzureBlob' || has(self.azureBlob)",message="azureBlob configuration is required when storageType is AzureBlob"
+// +kubebuilder:validation:XValidation:rule="!(has(self.s3) && has(self.azureBlob))",message="only one storage configuration should be provided"
 type HCPEtcdBackupSpec struct {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/hypershift/v1beta1/etcdbackup_types.go` around lines 51 - 68, Add
kubebuilder XValidation CEL rules on HCPEtcdBackupSpec to enforce the union
discriminator: validate that when StorageType == "S3" then S3 is non-null and
AzureBlob is null, when StorageType == "AzureBlob" then AzureBlob is non-null
and S3 is null, and ensure at most one of S3/AzureBlob is set. Apply the
XValidation annotations referencing the StorageType field and the pointer fields
S3 and AzureBlob so the API server enforces (1) correct config present for the
selected StorageType and (2) mutual exclusivity of S3 and AzureBlob for
HCPEtcdBackupSpec.
cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml (1)

159-168: Consider adding cross-field validation for storageType consistency.

The schema requires storageType but doesn't enforce that the corresponding storage configuration (s3 or azureBlob) is provided when a specific storageType is selected. A user could set storageType: S3 without providing the s3 configuration, or provide both s3 and azureBlob simultaneously.

Consider adding CEL validations at the spec level:

  • When storageType is S3, require s3 to be set and azureBlob to be unset.
  • When storageType is AzureBlob, require azureBlob to be set and s3 to be unset.
Example CEL validation to add at the spec level
x-kubernetes-validations:
- message: "s3 configuration is required when storageType is S3"
  rule: "self.storageType != 'S3' || has(self.s3)"
- message: "azureBlob configuration is required when storageType is AzureBlob"
  rule: "self.storageType != 'AzureBlob' || has(self.azureBlob)"
- message: "only one storage configuration should be provided"
  rule: "!(has(self.s3) && has(self.azureBlob))"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml`
around lines 159 - 168, Add cross-field CEL validations at the CRD spec level to
enforce consistency between storageType and its configuration fields: ensure
when storageType == "S3" that s3 is present and azureBlob is absent, when
storageType == "AzureBlob" that azureBlob is present and s3 is absent, and
forbid both s3 and azureBlob together; implement these as
x-kubernetes-validations entries referencing the spec (use the symbols
storageType, s3, azureBlob and x-kubernetes-validations) with clear messages for
each rule so the CRD rejects mismatched or duplicate storage configs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml`:
- Around line 3058-3069: Update the HCPEtcdBackupConfig Go struct to add
kubebuilder/CEL validation tags on the encryptionKeyURL and kmsKeyARN fields:
annotate encryptionKeyURL with a kubebuilder validation pattern that enforces an
https URL (e.g. +kubebuilder:validation:Pattern=`^https://[^/]+/.*$` or a
stricter URL regex) and annotate kmsKeyARN with a kubebuilder validation pattern
that matches AWS ARN format (e.g.
+kubebuilder:validation:Pattern=`^arn:aws:[a-z0-9-]+:[a-z0-9-]*:[0-9]*:[^\\s]+$`),
then regenerate the CRDs so the zz_generated.crd-manifests include these
CEL/schema checks; target the HCPEtcdBackupConfig type and the encryptionKeyURL
and kmsKeyARN field tags when making the change.

In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yaml`:
- Around line 3174-3189: The backup schema must validate provider-specific keys
and prevent mixed settings: add a kubebuilder/CRD pattern validation to
encryptionKeyURL (e.g., a regex matching Azure Key Vault key URLs like
https://<vault>.vault.azure.net/keys/<key>[/<version>]) and to kmsKeyARN (e.g.,
a regex matching AWS KMS ARNs like
arn:aws:kms:<region>:<account-id>:key/<key-id>), and add an
x-kubernetes-validations CEL rule on the backup object to reject payloads where
both encryptionKeyURL and kmsKeyARN are present (expression:
"!(has(self.encryptionKeyURL) && has(self.kmsKeyARN))" with an appropriate
message). Target the properties named encryptionKeyURL and kmsKeyARN and the
parent backup object in the CRD when making these changes.

---

Nitpick comments:
In `@api/hypershift/v1beta1/etcdbackup_types.go`:
- Around line 51-68: Add kubebuilder XValidation CEL rules on HCPEtcdBackupSpec
to enforce the union discriminator: validate that when StorageType == "S3" then
S3 is non-null and AzureBlob is null, when StorageType == "AzureBlob" then
AzureBlob is non-null and S3 is null, and ensure at most one of S3/AzureBlob is
set. Apply the XValidation annotations referencing the StorageType field and the
pointer fields S3 and AzureBlob so the API server enforces (1) correct config
present for the selected StorageType and (2) mutual exclusivity of S3 and
AzureBlob for HCPEtcdBackupSpec.

In
`@cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml`:
- Around line 159-168: Add cross-field CEL validations at the CRD spec level to
enforce consistency between storageType and its configuration fields: ensure
when storageType == "S3" that s3 is present and azureBlob is absent, when
storageType == "AzureBlob" that azureBlob is present and s3 is absent, and
forbid both s3 and azureBlob together; implement these as
x-kubernetes-validations entries referencing the spec (use the symbols
storageType, s3, azureBlob and x-kubernetes-validations) with clear messages for
each rule so the CRD rejects mismatched or duplicate storage configs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 36ae42cc-6d7f-4107-9a9c-ad6d9b6600ab

📥 Commits

Reviewing files that changed from the base of the PR and between 7604733 and dcf56ca.

⛔ Files ignored due to path filters (5)
  • api/hypershift/v1beta1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/zz_generated*
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/etcdbackup_types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
📒 Files selected for processing (17)
  • api/hypershift/v1beta1/etcdbackup_types.go
  • api/hypershift/v1beta1/featuregates/featureGate-Hypershift-Default.yaml
  • api/hypershift/v1beta1/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
  • api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml
  • api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
  • api/hypershift/v1beta1/hostedcluster_types.go
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hcpetcdbackups.hypershift.openshift.io/HCPEtcdBackup.yaml
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HCPEtcdBackup.yaml
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yaml
  • hypershift-operator/featuregate/feature.go
  • hypershift-operator/featuregate/feature_test.go

@cwbotbot

cwbotbot commented Mar 10, 2026

Copy link
Copy Markdown

Test Results

e2e-aws

Failed Tests

Total failed tests: 24

  • TestAutoscaling
  • TestAutoscaling/ValidateHostedCluster
  • TestCreateCluster
  • TestCreateCluster/ValidateHostedCluster
  • TestCreateClusterCustomConfig

... and 19 more failed tests

e2e-aks

Failed Tests

Total failed tests: 16

  • TestAutoscaling
  • TestAutoscaling/ValidateHostedCluster
  • TestAzureScheduler
  • TestAzureScheduler/ValidateHostedCluster
  • TestCreateCluster

... and 11 more failed tests

@openshift-ci-robot

openshift-ci-robot commented Mar 10, 2026

Copy link
Copy Markdown

@jparrill: This pull request references CNTRLPLANE-2677 which is a valid jira issue.

Details

In response to this:

Summary

  • Register the HCPEtcdBackup feature gate (disabled in Default, enabled in TechPreviewNoUpgrade)
  • Define the HCPEtcdBackup CRD API types (HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus, S3/AzureBlob storage configs, encryption metadata)
  • Add HCPEtcdBackupConfig to ManagedEtcdSpec for HC→HCP propagation of backup configuration (leveraging existing DeepCopy() path)
  • Generate deepcopy methods and featuregated CRD manifests via make hypershift-api

This is the first user story in the HCPEtcdBackup epic (CNTRLPLANE-2676). It defines the CRD types, feature gate, and propagation path. No controller logic is included.

JIRA

Test plan

  • go test ./hypershift-operator/featuregate/... — feature gate tests pass (HCPEtcdBackup disabled in Default, enabled in TechPreview)
  • make hypershift-api — generates deepcopy and CRD manifests without errors
  • go build ./... — full build compiles successfully
  • go test ./cmd/install/... — CRD install tests pass
  • Verified ManagedEtcdSpec.DeepCopyInto includes the new Backup field
  • Verified CRD manifests generated in cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
  • Added etcd backup capability with support for S3 and Azure Blob storage backends.
  • Introduced encryption configuration for backup artifacts using AWS KMS and Azure Key Vault (immutable once set).
  • New HCPEtcdBackup custom resource to manage backups and report status.
  • Feature-gated tech preview: HCPEtcdBackup can be enabled via feature gates and configurable on hosted cluster/control plane specs.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
api/hypershift/v1beta1/etcdbackup_types.go (1)

51-68: Consider adding CEL validation to enforce union member presence.

The union pattern uses storageType as discriminator but doesn't enforce that the corresponding storage configuration (s3 or azureBlob) is provided. A user could set storageType: S3 without providing the s3 field.

♻️ Proposed fix to add CEL validation
 // HCPEtcdBackupSpec defines the desired state of HCPEtcdBackup.
+// +kubebuilder:validation:XValidation:rule="self.storageType == 'S3' ? has(self.s3) : true",message="s3 configuration is required when storageType is S3"
+// +kubebuilder:validation:XValidation:rule="self.storageType == 'AzureBlob' ? has(self.azureBlob) : true",message="azureBlob configuration is required when storageType is AzureBlob"
 type HCPEtcdBackupSpec struct {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/hypershift/v1beta1/etcdbackup_types.go` around lines 51 - 68, Add CEL
validations on HCPEtcdBackupSpec to enforce the union: add two
x-kubernetes-validations (or equivalent kubebuilder marker) with CEL expressions
that when StorageType == "S3" then s3 must be present and when StorageType ==
"AzureBlob" then azureBlob must be present (e.g. self.storageType == "S3"
implies self.s3 != null and self.storageType == "AzureBlob" implies
self.azureBlob != null), referencing the StorageType, S3, and AzureBlob fields
so CRD validation rejects mismatched/missing union members.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/hypershift/v1beta1/etcdbackup_types.go`:
- Around line 119-124: The StorageAccount field validation in
etcdbackup_types.go currently uses +kubebuilder:validation:MinLength=1 which is
incorrect for Azure (must be 3–24 chars); update the StorageAccount struct tag
to use +kubebuilder:validation:MinLength=3 (leave MaxLength=24) so the
StorageAccount string field's kubebuilder validation matches Azure requirements
and regenerate CRDs if needed.

---

Nitpick comments:
In `@api/hypershift/v1beta1/etcdbackup_types.go`:
- Around line 51-68: Add CEL validations on HCPEtcdBackupSpec to enforce the
union: add two x-kubernetes-validations (or equivalent kubebuilder marker) with
CEL expressions that when StorageType == "S3" then s3 must be present and when
StorageType == "AzureBlob" then azureBlob must be present (e.g. self.storageType
== "S3" implies self.s3 != null and self.storageType == "AzureBlob" implies
self.azureBlob != null), referencing the StorageType, S3, and AzureBlob fields
so CRD validation rejects mismatched/missing union members.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6ec57230-00d2-4f88-97c8-13a65d34420b

📥 Commits

Reviewing files that changed from the base of the PR and between dcf56ca and 6d67ccb.

⛔ Files ignored due to path filters (5)
  • api/hypershift/v1beta1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/zz_generated*
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/etcdbackup_types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
📒 Files selected for processing (11)
  • api/hypershift/v1beta1/etcdbackup_types.go
  • api/hypershift/v1beta1/hostedcluster_types.go
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hcpetcdbackups.hypershift.openshift.io/HCPEtcdBackup.yaml
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HCPEtcdBackup.yaml
  • api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
  • api/hypershift/v1beta1/hostedcluster_types.go
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yaml
  • cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yaml

Comment thread api/hypershift/v1beta1/etcdbackup_types.go Outdated
@openshift-ci openshift-ci Bot added the area/documentation Indicates the PR includes changes for documentation label Mar 10, 2026
@openshift-ci-robot

openshift-ci-robot commented Mar 10, 2026

Copy link
Copy Markdown

@jparrill: This pull request references CNTRLPLANE-2677 which is a valid jira issue.

Details

In response to this:

Summary

  • Register the HCPEtcdBackup feature gate (disabled in Default, enabled in TechPreviewNoUpgrade)
  • Define the HCPEtcdBackup CRD API types (HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus, S3/AzureBlob storage configs, encryption metadata)
  • Add HCPEtcdBackupConfig to ManagedEtcdSpec for HC→HCP propagation of backup configuration (leveraging existing DeepCopy() path)
  • Generate deepcopy methods and featuregated CRD manifests via make hypershift-api
  • Add CEL validation on HCPEtcdBackupConfig to require at least one encryption key (kmsKeyARN or encryptionKeyURL). This prevents creating an empty config (backup: {}), which would make the inner immutable fields impossible to set later via update (since self == oldSelf would compare "" to the new value and reject it)

This is the first user story in the HCPEtcdBackup epic (CNTRLPLANE-2676). It defines the CRD types, feature gate, and propagation path. No controller logic is included.

JIRA

Test plan

  • go test ./hypershift-operator/featuregate/... — feature gate tests pass (HCPEtcdBackup disabled in Default, enabled in TechPreview)
  • make hypershift-api — generates deepcopy and CRD manifests without errors
  • go build ./... — full build compiles successfully
  • go test ./cmd/install/... — CRD install tests pass
  • Verified ManagedEtcdSpec.DeepCopyInto includes the new Backup field
  • Verified CRD manifests generated in cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/
  • make lint-fix && make verify — linter and verify checks pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
  • Added etcd backup capability with support for S3 and Azure Blob storage backends.
  • Introduced encryption configuration for backup artifacts using AWS KMS and Azure Key Vault (immutable once set).
  • New HCPEtcdBackup custom resource to manage backups and report status.
  • Feature-gated tech preview: HCPEtcdBackup can be enabled via feature gates and configurable on hosted cluster/control plane specs.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jparrill

Copy link
Copy Markdown
Contributor Author

Note on spec immutability

HCPEtcdBackup represents a one-shot backup request. Currently only the encryption fields (kmsKeyARN and encryptionKeyURL) are individually marked as immutable via XValidation:rule="self == oldSelf".

It might be worth considering making the entire spec immutable after creation in a follow-up, since modifying a backup request after it has been submitted (e.g., changing the bucket or region mid-flight) could lead to unexpected behavior. This would be a struct-level XValidation on HCPEtcdBackupSpec comparing self == oldSelf.

Leaving this as a note for future consideration — the current approach covers the critical fields and aligns with the enhancement spec.

@jparrill
jparrill force-pushed the CNTRLPLANE-2677 branch 2 times, most recently from 4270e08 to 68ec7e9 Compare March 10, 2026 11:10
Enable the HCPEtcdBackup feature gate so the new CRD can be
selectively activated per feature set. This is required before
defining the CRD API types.

Register the HCPEtcdBackup feature constant and variable in the
hypershift-operator featuregate package, gated behind
TechPreviewNoUpgrade. Update the four featuregate YAML manifests
(Hypershift and SelfManagedHA, Default and TechPreviewNoUpgrade)
and add unit tests covering all feature sets.

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Mar 18, 2026
@openshift-ci openshift-ci Bot added area/platform/aws PR/issue for AWS (AWSPlatform) platform and removed lgtm Indicates that a PR is ready to be merged. labels Mar 18, 2026
@sdminonne

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@openshift-ci-robot

Copy link
Copy Markdown

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@sdminonne

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@openshift-ci-robot

Copy link
Copy Markdown

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@jparrill

jparrill commented Mar 18, 2026

Copy link
Copy Markdown
Contributor Author

/retest-required

Infra issue

…h backup config

Why: The HCPEtcdBackup feature requires new API types for the CRD and
a backup configuration field in ManagedEtcdSpec to propagate encryption
settings from HostedCluster to HostedControlPlane.

How:
- Define HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus and
  supporting types (S3, AzureBlob, EncryptionMetadata) in
  etcdbackup_types.go with full validation markers.
- Add HCPEtcdBackupConfig with platform-specific blocks (AWS/Azure)
  and CEL mutual exclusion validation.
- Add Backup field to ManagedEtcdSpec behind HCPEtcdBackup feature gate.
- Enforce spec immutability via CEL (one-shot backup request).
- Enforce union discriminator via CEL (storageType must match backend).
- Add regexp validation for kmsKeyARN (^arn:) and encryptionKeyURL
  (^https://) following existing patterns in aws.go.
- Drop pointer for EncryptionMetadata in status (no nil vs zero
  semantic difference per OpenShift conventions).
- Regenerate deepcopy and CRD manifests via make hypershift-api.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@sdminonne

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2026
@openshift-ci-robot

Copy link
Copy Markdown

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-21
/test e2e-aws-4-21
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

@jparrill

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-self-managed

@jparrill

Copy link
Copy Markdown
Contributor Author

/verified by e2e

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 18, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@jparrill: This PR has been marked as verified by e2e.

Details

In response to this:

/verified by e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

@jparrill: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit b3974fe into openshift:main Mar 18, 2026
24 checks passed
mehabhalodiya pushed a commit to mehabhalodiya/hypershift that referenced this pull request Apr 13, 2026
…t#7898)

* feat(api): register HCPEtcdBackup feature gate

Enable the HCPEtcdBackup feature gate so the new CRD can be
selectively activated per feature set. This is required before
defining the CRD API types.

Register the HCPEtcdBackup feature constant and variable in the
hypershift-operator featuregate package, gated behind
TechPreviewNoUpgrade. Update the four featuregate YAML manifests
(Hypershift and SelfManagedHA, Default and TechPreviewNoUpgrade)
and add unit tests covering all feature sets.

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(api): add HCPEtcdBackup CRD types and extend ManagedEtcdSpec with backup config

Why: The HCPEtcdBackup feature requires new API types for the CRD and
a backup configuration field in ManagedEtcdSpec to propagate encryption
settings from HostedCluster to HostedControlPlane.

How:
- Define HCPEtcdBackup, HCPEtcdBackupSpec, HCPEtcdBackupStatus and
  supporting types (S3, AzureBlob, EncryptionMetadata) in
  etcdbackup_types.go with full validation markers.
- Add HCPEtcdBackupConfig with platform-specific blocks (AWS/Azure)
  and CEL mutual exclusion validation.
- Add Backup field to ManagedEtcdSpec behind HCPEtcdBackup feature gate.
- Enforce spec immutability via CEL (one-shot backup request).
- Enforce union discriminator via CEL (storageType must match backend).
- Add regexp validation for kmsKeyARN (^arn:) and encryptionKeyURL
  (^https://) following existing patterns in aws.go.
- Drop pointer for EncryptionMetadata in status (no nil vs zero
  semantic difference per OpenShift conventions).
- Regenerate deepcopy and CRD manifests via make hypershift-api.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>

---------

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/platform/aws PR/issue for AWS (AWSPlatform) platform jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants