Skip to content
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "DNS"
crdName: dnses.config.openshift.io
featureGates:
- AWSEuropeanSovereignCloudInstall
tests:
onCreate:
- name: Should be able to create a minimal DNS
initial: |
apiVersion: config.openshift.io/v1
kind: DNS
spec: {} # No spec is required for a DNS
expected: |
apiVersion: config.openshift.io/v1
kind: DNS
spec: {}
- name: Should be able to specify an AWS Europe Sovereign Cloud IAM role ARN for privateZoneIAMRole
initial: |
apiVersion: config.openshift.io/v1
kind: DNS
spec:
platform:
type: AWS
aws:
privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-role
expected: |
apiVersion: config.openshift.io/v1
kind: DNS
spec:
platform:
type: AWS
aws:
privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-role
- name: Should not be able to specify invalid privateZoneIAMRole ARN
initial: |
apiVersion: config.openshift.io/v1
kind: DNS
metadata:
name: cluster
spec:
platform:
type: AWS
aws:
privateZoneIAMRole: arn:invalid-partition:iam::123456789012:role/my-role
expectedError: "DNS.config.openshift.io \"cluster\" is invalid: spec.platform.aws.privateZoneIAMRole: Invalid value: \"arn:invalid-partition:iam::123456789012:role/my-role\": IAM role ARN must match \"^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\\/.*$\""
onUpdate:
- name: Can switch from empty (default), to AWS with aws-eusc IAM role ARN
initial: |
apiVersion: config.openshift.io/v1
kind: DNS
spec:
platform:
type: ""
updated: |
apiVersion: config.openshift.io/v1
kind: DNS
spec:
platform:
type: AWS
aws:
privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-role
expected: |
apiVersion: config.openshift.io/v1
kind: DNS
spec:
platform:
type: AWS
aws:
privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-role
3 changes: 2 additions & 1 deletion config/v1/types_dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ type AWSDNSSpec struct {
// privateZoneIAMRole contains the ARN of an IAM role that should be assumed when performing
// operations on the cluster's private hosted zone specified in the cluster DNS config.
// When left empty, no role should be assumed.
// +kubebuilder:validation:Pattern:=`^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$`
// +openshift:validation:FeatureGateAwareXValidation:featureGate="",rule=`matches(self, '^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$')`,message=`IAM role ARN must match "^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$"`
// +openshift:validation:FeatureGateAwareXValidation:featureGate=AWSEuropeanSovereignCloudInstall,rule=`matches(self, '^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\/.*$')`,message=`IAM role ARN must match "^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\/.*$"`
Comment on lines +137 to +138
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the file
fd "types_dns.go" --type f

Repository: openshift/api

Length of output: 104


🏁 Script executed:

# Read the specific lines in context
head -160 config/v1/types_dns.go | tail -40

Repository: openshift/api

Length of output: 2135


🏁 Script executed:

# Get more context around those lines to understand the field
sed -n '120,150p' config/v1/types_dns.go

Repository: openshift/api

Length of output: 1386


🏁 Script executed:

# Search for FeatureGateAwareXValidation in the codebase
rg "FeatureGateAwareXValidation" --type go -A 5 -B 5

Repository: openshift/api

Length of output: 40908


🏁 Script executed:

# Look for validation generation code
fd "*.go" --type f | xargs rg -l "FeatureGateAwareXValidation|featureGate" | head -10

Repository: openshift/api

Length of output: 784


🏁 Script executed:

# Check for any documentation or comments about the validation behavior
rg "featureGate.*replacement|featureGate.*additive|validation.*replacement" --type go -i

Repository: openshift/api

Length of output: 933


🏁 Script executed:

# Search for the FeatureGateXValidation type definition and ApplyToSchema implementation
rg -A 30 "type FeatureGateXValidation struct"

Repository: openshift/api

Length of output: 3854


🏁 Script executed:

# Search for code that processes or applies these markers to schema
rg "ApplyToSchema" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go -A 50 | head -100

Repository: openshift/api

Length of output: 4003


🏁 Script executed:

# Look for how x-kubernetes-validations are accumulated when multiple markers exist
rg "x-kubernetes-validations" -A 3 -B 3 --type go | head -60

Repository: openshift/api

Length of output: 5789


🏁 Script executed:

# Find the XValidation type and its ApplyToSchema method
rg -B 5 -A 20 "type XValidation struct" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go

Repository: openshift/api

Length of output: 39


🏁 Script executed:

# Find the ApplyToSchema for XValidation to see if it appends or replaces
rg -B 2 -A 20 "func (m XValidation) ApplyToSchema" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go

Repository: openshift/api

Length of output: 39


🏁 Script executed:

# Look for how multiple validations are handled - search for Validations field manipulation
rg "\.Validations = |\.Validations =" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go -B 3 -A 3

Repository: openshift/api

Length of output: 39


🏁 Script executed:

# Search for XValidation definition
rg "type XValidation" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/ -A 20

Repository: openshift/api

Length of output: 2497


🏁 Script executed:

# Search for how validation rules are applied
rg "Validations" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go -B 2 -A 2 | head -80

Repository: openshift/api

Length of output: 39


🏁 Script executed:

# Check the complete patch_validation.go file structure
wc -l tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go

Repository: openshift/api

Length of output: 138


🏁 Script executed:

# Read the entire patch_validation.go file to understand the full logic
cat tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/patch_validation.go

Repository: openshift/api

Length of output: 9963


🏁 Script executed:

# Also check the validation.go file for XValidation.ApplyToSchema
cat tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/validation.go | head -200

Repository: openshift/api

Length of output: 8081


🏁 Script executed:

# Find the XValidation.ApplyToSchema method
rg -A 30 "func (m XValidation) ApplyToSchema" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/validation.go

Repository: openshift/api

Length of output: 39


🏁 Script executed:

# Also check for how Validations field is used - look for append or assignment patterns
rg -B 5 -A 15 "XValidations" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/validation.go | head -60

Repository: openshift/api

Length of output: 788


🏁 Script executed:

# Confirm that multiple XValidations are indeed appended - look at the append pattern
rg "schema.XValidations = append" tools/vendor/sigs.k8s.io/controller-tools/pkg/crd/markers/ -B 3 -A 3

Repository: openshift/api

Length of output: 754


🏁 Script executed:

# Look for other examples of dual FeatureGateAwareXValidation in the codebase to understand the pattern
rg -B 3 "featureGate=\"\".*\n.*FeatureGateAwareXValidation:featureGate=" config/ | head -40

Repository: openshift/api

Length of output: 248


🏁 Script executed:

# Look for examples of dual FeatureGateAwareXValidation annotations on same field
rg -U "featureGate=\"\".*\n.*FeatureGateAwareXValidation" config/ -A 1 | head -80

Repository: openshift/api

Length of output: 1681


🏁 Script executed:

# Get a clearer picture - search for fields with multiple FeatureGateAwareXValidation markers
grep -r "FeatureGateAwareXValidation:featureGate=" config/ | grep -A 1 "featureGate=\"\"" | head -20

Repository: openshift/api

Length of output: 1262


🏁 Script executed:

# Check the HasAny implementation to understand empty featureGate behavior
rg -B 5 -A 10 "func.*HasAny" tools/vendor -A 5 | head -40

Repository: openshift/api

Length of output: 2736


🏁 Script executed:

# Check if there are any generated manifest files in this repo that show the actual output
fd "types_dns" --type f -path "*/config/*" | grep -E "(generated|manifest|openapi)"

Repository: openshift/api

Length of output: 225


The feature-gated DNS validator rules are both present in gate-enabled variants, not replaced.

Lines 137–138 both append validation rules to the same field. When the AWSEuropeanSovereignCloudInstall gate is enabled, both the base rule (rejecting aws-eusc) and the gate-enabled rule (accepting aws-eusc) are appended. Since all x-kubernetes-validations rules must pass, the base rule's rejection of aws-eusc takes precedence, making the gate ineffective. The architecture only supports additive validators; the gate-enabled rule must replace the base rule instead of coexisting with it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config/v1/types_dns.go` around lines 137 - 138, The unconditional validation
annotation that rejects aws-eusc (the line with
FeatureGateAwareXValidation:featureGate="") must be removed/converted so it does
not coexist with the gate-enabled validator; instead ensure there are two
mutually-exclusive validators for the same field: one that applies only when
AWSEuropeanSovereignCloudInstall is disabled (the rule that excludes aws-eusc)
and the other that applies only when AWSEuropeanSovereignCloudInstall is enabled
(the rule that includes aws-eusc). Update the FeatureGateAwareXValidation
annotations around the IAM role ARN (the two rules shown) so the base “reject
aws-eusc” rule is not unconditional but tied to the inverse/disabled state and
the gate-enabled rule (featureGate=AWSEuropeanSovereignCloudInstall) replaces it
when the gate is enabled, preventing both validators from being appended
simultaneously.

// +optional
PrivateZoneIAMRole string `json:"privateZoneIAMRole"`
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
api-approved.openshift.io: https://github.com/openshift/api/pull/470
api.openshift.io/merged-by-featuregates: "true"
include.release.openshift.io/ibm-cloud-managed: "true"
include.release.openshift.io/self-managed-high-availability: "true"
release.openshift.io/bootstrap-required: "true"
release.openshift.io/feature-set: CustomNoUpgrade
name: dnses.config.openshift.io
spec:
group: config.openshift.io
names:
kind: DNS
listKind: DNSList
plural: dnses
singular: dns
scope: Cluster
versions:
- name: v1
schema:
openAPIV3Schema:
description: |-
DNS holds cluster-wide information about DNS. The canonical name is `cluster`

Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
properties:
apiVersion:
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
kind:
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
metadata:
type: object
spec:
description: spec holds user settable values for configuration
properties:
baseDomain:
description: |-
baseDomain is the base domain of the cluster. All managed DNS records will
be sub-domains of this base.

For example, given the base domain `openshift.example.com`, an API server
DNS record may be created for `cluster-api.openshift.example.com`.

Once set, this field cannot be changed.
type: string
platform:
description: |-
platform holds configuration specific to the underlying
infrastructure provider for DNS.
When omitted, this means the user has no opinion and the platform is left
to choose reasonable defaults. These defaults are subject to change over time.
properties:
aws:
description: aws contains DNS configuration specific to the Amazon
Web Services cloud provider.
properties:
privateZoneIAMRole:
description: |-
privateZoneIAMRole contains the ARN of an IAM role that should be assumed when performing
operations on the cluster's private hosted zone specified in the cluster DNS config.
When left empty, no role should be assumed.
type: string
x-kubernetes-validations:
- message: IAM role ARN must match "^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$"
rule: matches(self, '^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$')
- message: IAM role ARN must match "^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\/.*$"
rule: matches(self, '^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\/.*$')
type: object
type:
description: |-
type is the underlying infrastructure provider for the cluster.
Allowed values: "", "AWS".

Individual components may not support all platforms,
and must handle unrecognized platforms with best-effort defaults.
enum:
- ""
- AWS
- Azure
- BareMetal
- GCP
- Libvirt
- OpenStack
- None
- VSphere
- oVirt
- IBMCloud
- KubeVirt
- EquinixMetal
- PowerVS
- AlibabaCloud
- Nutanix
- External
type: string
x-kubernetes-validations:
- message: allowed values are '' and 'AWS'
rule: self in ['','AWS']
required:
- type
type: object
x-kubernetes-validations:
- message: aws configuration is required when platform is AWS, and
forbidden otherwise
rule: 'has(self.type) && self.type == ''AWS'' ? has(self.aws) :
!has(self.aws)'
privateZone:
description: |-
privateZone is the location where all the DNS records that are only available internally
to the cluster exist.

If this field is nil, no private records should be created.

Once set, this field cannot be changed.
properties:
id:
description: |-
id is the identifier that can be used to find the DNS hosted zone.

on AWS zone can be fetched using `ID` as id in [1]
on Azure zone can be fetched using `ID` as a pre-determined name in [2],
on GCP zone can be fetched using `ID` as a pre-determined name in [3].

[1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
[2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
[3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get
type: string
tags:
additionalProperties:
type: string
description: |-
tags can be used to query the DNS hosted zone.

on AWS, resourcegroupstaggingapi [1] can be used to fetch a zone using `Tags` as tag-filters,

[1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options
type: object
type: object
publicZone:
description: |-
publicZone is the location where all the DNS records that are publicly accessible to
the internet exist.

If this field is nil, no public records should be created.

Once set, this field cannot be changed.
properties:
id:
description: |-
id is the identifier that can be used to find the DNS hosted zone.

on AWS zone can be fetched using `ID` as id in [1]
on Azure zone can be fetched using `ID` as a pre-determined name in [2],
on GCP zone can be fetched using `ID` as a pre-determined name in [3].

[1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
[2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
[3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get
type: string
tags:
additionalProperties:
type: string
description: |-
tags can be used to query the DNS hosted zone.

on AWS, resourcegroupstaggingapi [1] can be used to fetch a zone using `Tags` as tag-filters,

[1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options
type: object
type: object
type: object
status:
description: status holds observed values from the cluster. They may not
be overridden.
type: object
required:
- spec
type: object
served: true
storage: true
subresources:
status: {}
Loading