Skip to content

NE-2411: Add Template field to DNS operator#2765

Open
grzpiotrowski wants to merge 1 commit intoopenshift:masterfrom
grzpiotrowski:NE-2411
Open

NE-2411: Add Template field to DNS operator#2765
grzpiotrowski wants to merge 1 commit intoopenshift:masterfrom
grzpiotrowski:NE-2411

Conversation

@grzpiotrowski
Copy link
Contributor

No description provided.

@openshift-ci-robot
Copy link

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
Copy link
Contributor

openshift-ci bot commented Mar 13, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@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 13, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 13, 2026

Hello @grzpiotrowski! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 13, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 86e2be6d-7cda-4685-949c-3577f9ad42cb

📥 Commits

Reviewing files that changed from the base of the PR and between 23bfcb6 and a7d69ac.

⛔ Files ignored due to path filters (7)
  • openapi/generated_openapi/zz_generated.openapi.go is excluded by !openapi/**
  • operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses-CustomNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses-Default.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses-DevPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses-OKD.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses-TechPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • operator/v1/zz_generated.featuregated-crd-manifests/dnses.operator.openshift.io/DNSTemplatePlugin.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (14)
  • features.md
  • features/features.go
  • operator/v1/types_dns.go
  • operator/v1/zz_generated.deepcopy.go
  • operator/v1/zz_generated.featuregated-crd-manifests.yaml
  • operator/v1/zz_generated.swagger_doc_generated.go
  • payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml
  • payload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yaml
  • payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
  • payload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yaml
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
✅ Files skipped from review due to trivial changes (5)
  • payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
  • features.md
  • payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
  • operator/v1/zz_generated.swagger_doc_generated.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • features/features.go
  • operator/v1/types_dns.go

📝 Walkthrough

Walkthrough

Adds a new feature gate DNSTemplatePlugin and wires it into feature gate registration and payload manifests (enabled in DevPreviewNoUpgrade variants, disabled in others). Extends the DNS API with an optional Template field on DNSSpec and new types (Template, TemplateAction, ReturnEmptyAction, QueryType, QueryClass, ResponseCode). Updates generated deepcopy and Swagger docs and marks the CRD as feature-gated for DNSTemplatePlugin.

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate relatedness to the changeset. Add a description explaining the purpose, motivation, and details of the templates field addition to the DNS operator.
✅ Passed checks (7 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names ✅ Passed This pull request does not contain any Ginkgo test files or test patterns. The modifications are limited to feature gate and DNS type definitions.
Test Structure And Quality ✅ Passed The pull request contains only API type definitions and feature gate declarations, with no Ginkgo test code present, making this check not applicable.
Microshift Test Compatibility ✅ Passed The pull request does not introduce any new Ginkgo e2e tests. The changes consist exclusively of API/CRD type definitions: a new feature gate variable in features/features.go and new DNS template types in operator/v1/types_dns.go. No test files were added or modified according to the git diff. Since the MicroShift Test Compatibility check applies only to new Ginkgo e2e tests (It(), Describe(), Context(), When(), etc.), and this PR contains no such tests, the check is not applicable and therefore passes.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request does not introduce any new Ginkgo e2e tests. The PR only modifies two API definition files: features/features.go and operator/v1/types_dns.go, neither containing test code patterns.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This pull request does not introduce any new Ginkgo e2e tests. The changes consist solely of API type definitions and a feature gate declaration. Since no test files or e2e test functions are present, the check is not applicable.
Title check ✅ Passed The title accurately describes the main change: adding a templates field to the DNS operator, which is reflected in both the feature gate introduction and the new Templates field in DNSSpec.

✏️ 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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented
The command is terminated due to an error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure the reviews.tools.github-checks setting to adjust the time to wait for GitHub Checks to complete.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign everettraven for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@grzpiotrowski
Copy link
Contributor Author

grzpiotrowski commented Mar 13, 2026

/retitle [WIP] NE-2411: Add templates field to DNS operator

@openshift-ci openshift-ci bot changed the title [WIP] Add templates field to DNS operator [WIP] NE-2411: Add templates field to DNS operator Mar 13, 2026
@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 13, 2026
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 13, 2026

@grzpiotrowski: This pull request references NE-2411 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:

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@operator/v1/types_dns.go`:
- Around line 528-535: The Zones slice lacks per-item format validation; update
the Zones field in operator/v1/types_dns.go to add a kubebuilder validation
Pattern that enforces RFC1123 DNS names or the literal ".". Specifically, add a
comment like //
+kubebuilder:validation:Pattern="^(\\.|[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)$"
immediately above the Zones []string `json:"zones"` declaration so each entry is
validated as either "." or an RFC1123-compliant name. Ensure the annotation is
placed alongside the existing Required/MinItems tags.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 964b4669-8aa2-4d3f-bc4d-79f2a6924c53

📥 Commits

Reviewing files that changed from the base of the PR and between 5e946e2 and 23bfcb6.

📒 Files selected for processing (2)
  • features/features.go
  • operator/v1/types_dns.go

Comment on lines +528 to +535
// zones specifies the DNS zones this template applies to.
// Each zone must be a valid DNS name as defined in RFC 1123.
// The special zone "." matches all domains (catch-all).
// At least one zone must be specified.
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinItems=1
Zones []string `json:"zones"`

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add schema validation for zones item format.

The comments require RFC1123-compatible zones (plus "."), but the schema currently only enforces presence/count. That allows malformed zones to be persisted and fail downstream.

Suggested CRD validation guard
 type Template struct {
 	// zones specifies the DNS zones this template applies to.
 	// Each zone must be a valid DNS name as defined in RFC 1123.
 	// The special zone "." matches all domains (catch-all).
 	// At least one zone must be specified.
+	// +kubebuilder:validation:items:Pattern=`^(\.|([a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.([a-z0-9]([-a-z0-9]*[a-z0-9])?))*)$`
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:MinItems=1
 	Zones []string `json:"zones"`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// zones specifies the DNS zones this template applies to.
// Each zone must be a valid DNS name as defined in RFC 1123.
// The special zone "." matches all domains (catch-all).
// At least one zone must be specified.
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinItems=1
Zones []string `json:"zones"`
// zones specifies the DNS zones this template applies to.
// Each zone must be a valid DNS name as defined in RFC 1123.
// The special zone "." matches all domains (catch-all).
// At least one zone must be specified.
// +kubebuilder:validation:items:Pattern=`^(\.|([a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.([a-z0-9]([-a-z0-9]*[a-z0-9])?))*)$`
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinItems=1
Zones []string `json:"zones"`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@operator/v1/types_dns.go` around lines 528 - 535, The Zones slice lacks
per-item format validation; update the Zones field in operator/v1/types_dns.go
to add a kubebuilder validation Pattern that enforces RFC1123 DNS names or the
literal ".". Specifically, add a comment like //
+kubebuilder:validation:Pattern="^(\\.|[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)$"
immediately above the Zones []string `json:"zones"` declaration so each entry is
validated as either "." or an RFC1123-compliant name. Ensure the annotation is
placed alongside the existing Required/MinItems tags.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 18, 2026
@openshift-ci openshift-ci bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 19, 2026
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2026
@grzpiotrowski grzpiotrowski changed the title [WIP] NE-2411: Add templates field to DNS operator NE-2411: Add templates field to DNS operator Mar 20, 2026
@grzpiotrowski grzpiotrowski marked this pull request as ready for review March 20, 2026 11:11
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 20, 2026
@grzpiotrowski grzpiotrowski changed the title NE-2411: Add templates field to DNS operator NE-2411: Add Template field to DNS operator Mar 20, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 20, 2026

@grzpiotrowski: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/lint a7d69ac link true /test lint
ci/prow/verify a7d69ac link true /test verify
ci/prow/verify-crd-schema a7d69ac link true /test verify-crd-schema

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants