Skip to content

Commit e8e07fa

Browse files
Merge pull request #2133 from JoelSpeed/require-multiple-featuregates
Fix handling of validations that require multiple feature gates
2 parents c21a6eb + e8a4d21 commit e8e07fa

23 files changed

+381
-24
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
2+
name: "Example API"
3+
crdName: stableconfigtypes.example.openshift.io
4+
featureGates:
5+
- Example
6+
- Example2
7+
tests:
8+
onCreate:
9+
- name: Should persist stable fields
10+
initial: |
11+
apiVersion: example.openshift.io/v1
12+
kind: StableConfigType
13+
spec:
14+
stableField: "Allowed"
15+
immutableField: foo
16+
expected: |
17+
apiVersion: example.openshift.io/v1
18+
kind: StableConfigType
19+
spec:
20+
stableField: "Allowed"
21+
immutableField: foo
22+
nonZeroDefault: 8
23+
onUpdate:
24+
- name: Should not allow removal of a tech preview field
25+
initial: |
26+
apiVersion: example.openshift.io/v1
27+
kind: StableConfigType
28+
spec:
29+
stableField: "Invalid"
30+
immutableField: foo
31+
updated: |
32+
apiVersion: example.openshift.io/v1
33+
kind: StableConfigType
34+
spec:
35+
immutableField: foo
36+
expectedError: "spec: Invalid value: \"object\": stableField may not be removed once set"

example/v1/types_stable.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ type StableConfigType struct {
3333

3434
// StableConfigTypeSpec is the desired state
3535
// +openshift:validation:FeatureGateAwareXValidation:featureGate=Example,rule="has(oldSelf.coolNewField) ? has(self.coolNewField) : true",message="coolNewField may not be removed once set"
36+
// +openshift:validation:FeatureGateAwareXValidation:requiredFeatureGate=Example;Example2,rule="has(oldSelf.stableField) ? has(self.stableField) : true",message="stableField may not be removed once set (this should only show up with both the Example and Example2 feature gates)"
3637
type StableConfigTypeSpec struct {
3738
// coolNewField is a field that is for tech preview only. On normal clusters this shouldn't be present
3839
//

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ spec:
168168
x-kubernetes-validations:
169169
- message: coolNewField may not be removed once set
170170
rule: 'has(oldSelf.coolNewField) ? has(self.coolNewField) : true'
171+
- message: stableField may not be removed once set (this should only show
172+
up with both the Example and Example2 feature gates)
173+
rule: 'has(oldSelf.stableField) ? has(self.stableField) : true'
171174
status:
172175
description: status is the most recently observed status of the StableConfigType.
173176
properties:

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-DevPreviewNoUpgrade.crd.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ spec:
168168
x-kubernetes-validations:
169169
- message: coolNewField may not be removed once set
170170
rule: 'has(oldSelf.coolNewField) ? has(self.coolNewField) : true'
171+
- message: stableField may not be removed once set (this should only show
172+
up with both the Example and Example2 feature gates)
173+
rule: 'has(oldSelf.stableField) ? has(self.stableField) : true'
171174
status:
172175
description: status is the most recently observed status of the StableConfigType.
173176
properties:

example/v1/zz_generated.featuregated-crd-manifests.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ stableconfigtypes.example.openshift.io:
66
Category: ""
77
FeatureGates:
88
- Example
9+
- Example+Example2
910
FilenameOperatorName: my-operator
1011
FilenameOperatorOrdering: "01"
1112
FilenameRunLevel: "0000_50"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
annotations:
5+
api-approved.openshift.io: https://github.com/openshift/api/pull/xxx
6+
api.openshift.io/filename-cvo-runlevel: "0000_50"
7+
api.openshift.io/filename-operator: my-operator
8+
api.openshift.io/filename-ordering: "01"
9+
feature-gate.release.openshift.io/Example: "true"
10+
feature-gate.release.openshift.io/Example2: "true"
11+
name: stableconfigtypes.example.openshift.io
12+
spec:
13+
group: example.openshift.io
14+
names:
15+
kind: StableConfigType
16+
listKind: StableConfigTypeList
17+
plural: stableconfigtypes
18+
singular: stableconfigtype
19+
scope: Cluster
20+
versions:
21+
- name: v1
22+
schema:
23+
openAPIV3Schema:
24+
description: |-
25+
StableConfigType is a stable config type that may include TechPreviewNoUpgrade fields.
26+
27+
Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
28+
properties:
29+
apiVersion:
30+
description: |-
31+
APIVersion defines the versioned schema of this representation of an object.
32+
Servers should convert recognized schemas to the latest internal value, and
33+
may reject unrecognized values.
34+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
35+
type: string
36+
kind:
37+
description: |-
38+
Kind is a string value representing the REST resource this object represents.
39+
Servers may infer this from the endpoint the client submits requests to.
40+
Cannot be updated.
41+
In CamelCase.
42+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
43+
type: string
44+
metadata:
45+
type: object
46+
spec:
47+
description: spec is the specification of the desired behavior of the
48+
StableConfigType.
49+
properties:
50+
celUnion:
51+
description: celUnion demonstrates how to validate a discrminated
52+
union using CEL
53+
properties:
54+
optionalMember:
55+
description: optionalMember is a union member that is optional.
56+
type: string
57+
requiredMember:
58+
description: requiredMember is a union member that is required.
59+
type: string
60+
type:
61+
description: type determines which of the union members should
62+
be populated.
63+
enum:
64+
- RequiredMember
65+
- OptionalMember
66+
- EmptyMember
67+
type: string
68+
required:
69+
- type
70+
type: object
71+
x-kubernetes-validations:
72+
- message: requiredMember is required when type is RequiredMember,
73+
and forbidden otherwise
74+
rule: 'has(self.type) && self.type == ''RequiredMember'' ? has(self.requiredMember)
75+
: !has(self.requiredMember)'
76+
- message: optionalMember is forbidden when type is not OptionalMember
77+
rule: 'has(self.type) && self.type == ''OptionalMember'' ? true
78+
: !has(self.optionalMember)'
79+
coolNewField:
80+
description: coolNewField is a field that is for tech preview only. On
81+
normal clusters this shouldn't be present
82+
type: string
83+
evolvingCollection:
84+
description: |-
85+
evolvingCollection demonstrates how to have a collection where the maximum number of items varies on cluster type.
86+
For default clusters, this will be "1" but on TechPreview clusters, this value will be "3".
87+
items:
88+
type: string
89+
maxItems: 3
90+
type: array
91+
x-kubernetes-list-type: atomic
92+
evolvingUnion:
93+
description: evolvingUnion demonstrates how to phase in new values
94+
into discriminated union
95+
properties:
96+
type:
97+
description: type is the discriminator. It has different values
98+
for Default and for TechPreviewNoUpgrade
99+
enum:
100+
- ""
101+
- StableValue
102+
- TechPreviewOnlyValue
103+
type: string
104+
required:
105+
- type
106+
type: object
107+
immutableField:
108+
description: |-
109+
immutableField is a field that is immutable once the object has been created.
110+
It is required at all times.
111+
type: string
112+
x-kubernetes-validations:
113+
- message: immutableField is immutable
114+
rule: self == oldSelf
115+
nonZeroDefault:
116+
default: 8
117+
description: |-
118+
nonZeroDefault is a demonstration of creating an integer field that has a non zero default.
119+
It required two default tags (one for CRD generation, one for client generation) and must have `omitempty` and be optional.
120+
A minimum value is added to demonstrate that a zero value would not be accepted.
121+
format: int32
122+
minimum: 8
123+
type: integer
124+
optionalImmutableField:
125+
description: |-
126+
optionalImmutableField is a field that is immutable once set.
127+
It is optional but may not be changed once set.
128+
type: string
129+
x-kubernetes-validations:
130+
- message: optionalImmutableField is immutable once set
131+
rule: oldSelf == '' || self == oldSelf
132+
set:
133+
description: set demonstrates how to define and validate set of strings
134+
items:
135+
description: SetValue defines the types allowed in string set type
136+
enum:
137+
- Foo
138+
- Bar
139+
- Baz
140+
- Qux
141+
- Corge
142+
type: string
143+
maxItems: 5
144+
type: array
145+
x-kubernetes-list-type: set
146+
x-kubernetes-validations:
147+
- rule: self.all(x,self.exists_one(y,x == y))
148+
stableField:
149+
description: |-
150+
stableField is a field that is present on default clusters and on tech preview clusters
151+
152+
If empty, the platform will choose a good default, which may change over time without notice.
153+
type: string
154+
subdomainNameField:
155+
description: |-
156+
subdomainNameField represents a kubenetes name field.
157+
The intention is that it validates the name in the same way metadata.Name is validated.
158+
That is, it is a DNS-1123 subdomain.
159+
maxLength: 253
160+
type: string
161+
x-kubernetes-validations:
162+
- message: a lowercase RFC 1123 subdomain must consist of lower case
163+
alphanumeric characters, '-' or '.', and must start and end with
164+
an alphanumeric character.
165+
rule: '!format.dns1123Subdomain().validate(self).hasValue()'
166+
required:
167+
- immutableField
168+
type: object
169+
x-kubernetes-validations:
170+
- message: coolNewField may not be removed once set
171+
rule: 'has(oldSelf.coolNewField) ? has(self.coolNewField) : true'
172+
- message: stableField may not be removed once set (this should only show
173+
up with both the Example and Example2 feature gates)
174+
rule: 'has(oldSelf.stableField) ? has(self.stableField) : true'
175+
status:
176+
description: status is the most recently observed status of the StableConfigType.
177+
properties:
178+
conditions:
179+
description: |-
180+
Represents the observations of a foo's current state.
181+
Known .status.conditions.type are: "Available", "Progressing", and "Degraded"
182+
items:
183+
description: Condition contains details for one aspect of the current
184+
state of this API Resource.
185+
properties:
186+
lastTransitionTime:
187+
description: |-
188+
lastTransitionTime is the last time the condition transitioned from one status to another.
189+
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
190+
format: date-time
191+
type: string
192+
message:
193+
description: |-
194+
message is a human readable message indicating details about the transition.
195+
This may be an empty string.
196+
maxLength: 32768
197+
type: string
198+
observedGeneration:
199+
description: |-
200+
observedGeneration represents the .metadata.generation that the condition was set based upon.
201+
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
202+
with respect to the current state of the instance.
203+
format: int64
204+
minimum: 0
205+
type: integer
206+
reason:
207+
description: |-
208+
reason contains a programmatic identifier indicating the reason for the condition's last transition.
209+
Producers of specific condition types may define expected values and meanings for this field,
210+
and whether the values are considered a guaranteed API.
211+
The value should be a CamelCase string.
212+
This field may not be empty.
213+
maxLength: 1024
214+
minLength: 1
215+
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
216+
type: string
217+
status:
218+
description: status of the condition, one of True, False, Unknown.
219+
enum:
220+
- "True"
221+
- "False"
222+
- Unknown
223+
type: string
224+
type:
225+
description: type of condition in CamelCase or in foo.example.com/CamelCase.
226+
maxLength: 316
227+
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
228+
type: string
229+
required:
230+
- lastTransitionTime
231+
- message
232+
- reason
233+
- status
234+
- type
235+
type: object
236+
type: array
237+
x-kubernetes-list-map-keys:
238+
- type
239+
x-kubernetes-list-type: map
240+
immutableField:
241+
description: |-
242+
immutableField is a field that is immutable once the object has been created.
243+
It is required at all times.
244+
type: string
245+
x-kubernetes-validations:
246+
- message: immutableField is immutable
247+
rule: self == oldSelf
248+
type: object
249+
type: object
250+
served: true
251+
storage: true
252+
subresources:
253+
status: {}

features.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
| MachineAPIOperatorDisableMachineHealthCheckController| | | | | | |
88
| MultiArchInstallAzure| | | | | | |
99
| ClusterVersionOperatorConfiguration| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | |
10+
| Example2| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | |
1011
| GatewayAPI| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | |
1112
| NewOLM| | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> |
1213
| AWSClusterHostedDNS| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |

features/features.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,14 @@ var (
498498
enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
499499
mustRegister()
500500

501+
FeatureGateExample2 = newFeatureGate("Example2").
502+
reportProblemsToJiraComponent("cluster-config").
503+
contactPerson("JoelSpeed").
504+
productScope(ocpSpecific).
505+
enhancementPR(legacyFeatureGateWithoutEnhancement).
506+
enableIn(configv1.DevPreviewNoUpgrade).
507+
mustRegister()
508+
501509
FeatureGatePlatformOperators = newFeatureGate("PlatformOperators").
502510
reportProblemsToJiraComponent("olm").
503511
contactPerson("joe").

features/legacyfeaturegates.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ var legacyFeatureGates = sets.New(
3737
// never add to this list, if you think you have an exception ask @deads2k
3838
"Example",
3939
// never add to this list, if you think you have an exception ask @deads2k
40+
"Example2",
41+
// never add to this list, if you think you have an exception ask @deads2k
4042
"GCPClusterHostedDNS",
4143
// never add to this list, if you think you have an exception ask @deads2k
4244
"GCPLabelsTags",

payload-command/render/legacyfeaturegates.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ var legacyFeatureGates = sets.New(
3939
// never add to this list, if you think you have an exception ask @deads2k
4040
"Example",
4141
// never add to this list, if you think you have an exception ask @deads2k
42+
"Example2",
43+
// never add to this list, if you think you have an exception ask @deads2k
4244
"GCPClusterHostedDNS",
4345
// never add to this list, if you think you have an exception ask @deads2k
4446
"GCPLabelsTags",

payload-manifests/featuregates/featureGate-Hypershift-Default.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@
5555
{
5656
"name": "Example"
5757
},
58+
{
59+
"name": "Example2"
60+
},
5861
{
5962
"name": "GCPClusterHostedDNS"
6063
},

payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@
101101
{
102102
"name": "Example"
103103
},
104+
{
105+
"name": "Example2"
106+
},
104107
{
105108
"name": "ExternalOIDC"
106109
},

payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
{
2828
"name": "EventedPLEG"
2929
},
30+
{
31+
"name": "Example2"
32+
},
3033
{
3134
"name": "GatewayAPI"
3235
},

0 commit comments

Comments
 (0)