Skip to content

Commit 6674623

Browse files
itzPranshulitzPranshul
committed
feat: implement +k8s:opaque declarative validation marker and add tests
1 parent c759887 commit 6674623

5 files changed

Lines changed: 158 additions & 6 deletions

File tree

‎pkg/crd/markers/validation.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ const (
3535
ValidationExactlyOneOfPrefix = validationPrefix + "ExactlyOneOf"
3636
ValidationAtMostOneOfPrefix = validationPrefix + "AtMostOneOf"
3737
ValidationAtLeastOneOfPrefix = validationPrefix + "AtLeastOneOf"
38+
39+
OpaqueFieldName = "k8s:opaque"
3840
)
3941

4042
// ValidationMarkers lists all available markers that affect CRD schema generation,
@@ -123,6 +125,9 @@ var FieldOnlyMarkers = []*definitionWithHelp{
123125

124126
must(markers.MakeDefinition(SchemalessName, markers.DescribesField, Schemaless{})).
125127
WithHelp(Schemaless{}.Help()),
128+
129+
must(markers.MakeDefinition(OpaqueFieldName, markers.DescribesField, Opaque{})).
130+
WithHelp(markers.SimpleHelp("CRD validation", "suppresses type-level validation inheritance for this field; field-level markers still apply.")),
126131
}
127132

128133
// ValidationIshMarkers are field-and-type markers that don't fall under the
@@ -568,6 +573,10 @@ type XIntOrString struct{}
568573
// +controllertools:marker:generateHelp:category="CRD validation"
569574
type Schemaless struct{}
570575

576+
// Opaque instructs the CRD generator to suppress inheritance of type-level
577+
// validation for this field. Field-level markers still apply.
578+
type Opaque struct{}
579+
571580
func hasNumericType(schema *apiextensionsv1.JSONSchemaProps) bool {
572581
return schema.Type == string(Integer) || schema.Type == string(Number)
573582
}

‎pkg/crd/schema.go‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ type schemaContext struct {
7373

7474
allowDangerousTypes bool
7575
ignoreUnexportedFields bool
76+
opaque bool
7677
}
7778

7879
// newSchemaContext constructs a new schemaContext for the given package and schema requester.
@@ -99,6 +100,16 @@ func (c *schemaContext) ForInfo(info *markers.TypeInfo) *schemaContext {
99100
}
100101
}
101102

103+
// ForInfoOpaque produces a new schemaContext with containing the same information
104+
// as this one, except with the given type information and marked as opaque.
105+
// An opaque context prevents the emission of $ref to the type's schema, suppressing
106+
// inherited type-level validations.
107+
func (c *schemaContext) ForInfoOpaque(info *markers.TypeInfo) *schemaContext {
108+
ctx := c.ForInfo(info)
109+
ctx.opaque = true
110+
return ctx
111+
}
112+
102113
// requestSchema asks for the schema for a type in the package with the
103114
// given import path.
104115
func (c *schemaContext) requestSchema(pkgPath, typeName string) {
@@ -285,6 +296,12 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
285296
// > Otherwise, the alias information is only in the type name, which
286297
// > points directly to the actual (aliased) type.
287298
if typeInfo.Name() != ident.Name {
299+
if ctx.opaque {
300+
return &apiextensionsv1.JSONSchemaProps{
301+
Type: typ,
302+
Format: fmt,
303+
}
304+
}
288305
ctx.requestSchema("", ident.Name)
289306
link := TypeRefLink("", ident.Name)
290307
return &apiextensionsv1.JSONSchemaProps{
@@ -309,8 +326,6 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
309326
if pkg == ctx.pkg.Types {
310327
pkgPath = ""
311328
}
312-
ctx.requestSchema(pkgPath, typeNameInfo.Name())
313-
link := TypeRefLink(pkgPath, typeNameInfo.Name())
314329

315330
// In cases where we have a named type, apply the type and format from the named schema
316331
// to allow markers that need this information to apply correctly.
@@ -327,11 +342,18 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
327342
}
328343
}
329344

330-
return &apiextensionsv1.JSONSchemaProps{
345+
props := &apiextensionsv1.JSONSchemaProps{
331346
Type: typ,
332347
Format: fmt,
333-
Ref: &link,
334348
}
349+
350+
if !ctx.opaque {
351+
ctx.requestSchema(pkgPath, typeNameInfo.Name())
352+
link := TypeRefLink(pkgPath, typeNameInfo.Name())
353+
props.Ref = &link
354+
}
355+
356+
return props
335357
default:
336358
ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("unsupported type %T for identifier %s", typeInfo, ident.Name), ident))
337359
return &apiextensionsv1.JSONSchemaProps{}
@@ -537,9 +559,12 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiextensio
537559
}
538560

539561
var propSchema *apiextensionsv1.JSONSchemaProps
540-
if field.Markers.Get(crdmarkers.SchemalessName) != nil {
562+
switch {
563+
case field.Markers.Get(crdmarkers.SchemalessName) != nil:
541564
propSchema = &apiextensionsv1.JSONSchemaProps{}
542-
} else {
565+
case field.Markers.Get(crdmarkers.OpaqueFieldName) != nil:
566+
propSchema = typeToSchema(ctx.ForInfoOpaque(&markers.TypeInfo{}), field.RawField.Type)
567+
default:
543568
propSchema = typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), field.RawField.Type)
544569
}
545570
propSchema.Description = field.Doc

‎pkg/crd/testdata/crd_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,15 @@ limitations under the License.
1616
package cronjob
1717

1818
import (
19+
"context"
1920
"os"
21+
"time"
2022

2123
. "github.com/onsi/ginkgo/v2"
2224
. "github.com/onsi/gomega"
2325
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
26+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
27+
"k8s.io/apimachinery/pkg/runtime/schema"
2428
"sigs.k8s.io/yaml"
2529
)
2630

@@ -36,4 +40,98 @@ var _ = Describe("CronJob CRD", func() {
3640
err = k8sClient.Create(ctx, crd)
3741
Expect(err).NotTo(HaveOccurred())
3842
})
43+
44+
Context("validating opaque markers", func() {
45+
applyCronJob := func(ctx context.Context, name, opaqueVal, nonOpaqueVal string) error {
46+
obj := &unstructured.Unstructured{}
47+
obj.SetGroupVersionKind(schema.GroupVersionKind{
48+
Group: "testdata.kubebuilder.io",
49+
Version: "v1",
50+
Kind: "CronJob",
51+
})
52+
obj.SetName(name)
53+
obj.SetNamespace("default")
54+
55+
spec := map[string]interface{}{
56+
"schedule": "*/5 * * * *", // required field
57+
"foo": "bar",
58+
"baz": "baz",
59+
"binaryName": "YmluYXJ5", // base64 for "binary"
60+
"canBeNull": "ok",
61+
"defaultedEmptyMap": map[string]interface{}{},
62+
"defaultedEmptyObject": map[string]interface{}{},
63+
"defaultedEmptySlice": []interface{}{},
64+
"defaultedObject": []interface{}{},
65+
"defaultedSlice": []interface{}{},
66+
"defaultedString": "some string",
67+
"doubleDefaultedString": "some string",
68+
"embeddedResource": map[string]interface{}{"kind": "Pod", "apiVersion": "v1"},
69+
"explicitlyRequiredK8s": "required",
70+
"explicitlyRequiredKubebuilder": "required",
71+
"explicitlyRequiredKubernetes": "required",
72+
"float64WithValidations": 1.5,
73+
"floatWithValidations": 1.5,
74+
"int32WithValidations": 2,
75+
"intWithValidations": 2,
76+
"jobTemplate": map[string]interface{}{
77+
"template": map[string]interface{}{},
78+
},
79+
"kubernetesDefaultedEmptyMap": map[string]interface{}{},
80+
"kubernetesDefaultedEmptyObject": map[string]interface{}{},
81+
"kubernetesDefaultedEmptySlice": []interface{}{},
82+
"kubernetesDefaultedObject": []interface{}{},
83+
"kubernetesDefaultedSlice": []interface{}{},
84+
"kubernetesDefaultedString": "string",
85+
"mapOfInfo": map[string]interface{}{},
86+
"nestedMapOfInfo": map[string]interface{}{},
87+
"nestedStructWithSeveralFields": map[string]interface{}{"foo": "str", "bar": true},
88+
"nestedStructWithSeveralFieldsDoubleMarked": map[string]interface{}{"foo": "str", "bar": true},
89+
"nestedassociativeList": []interface{}{},
90+
"patternObject": "https://example.com",
91+
"stringPair": []interface{}{"a", "b"},
92+
"structWithSeveralFields": map[string]interface{}{"foo": "str", "bar": true},
93+
"twoOfAKindPart0": "longenough",
94+
"twoOfAKindPart1": "longenough",
95+
"unprunedEmbeddedResource": map[string]interface{}{"kind": "Pod", "apiVersion": "v1"},
96+
"unprunedFomType": map[string]interface{}{},
97+
"unprunedFomTypeAndField": map[string]interface{}{},
98+
"unprunedJSON": map[string]interface{}{"foo": "str", "bar": true},
99+
"associativeList": []interface{}{},
100+
}
101+
if opaqueVal != "" {
102+
spec["opaqueField"] = opaqueVal
103+
}
104+
if nonOpaqueVal != "" {
105+
spec["nonOpaqueField"] = nonOpaqueVal
106+
}
107+
obj.Object["spec"] = spec
108+
109+
return k8sClient.Create(ctx, obj)
110+
}
111+
112+
It("should suppress type-level validation for fields with +k8s:opaque", func(ctx SpecContext) {
113+
// type-level validation is MinLength=4
114+
// field-level validation is MaxLength=5
115+
116+
By("allowing opaqueField with length 3 (suppresses type-level MinLength=4)")
117+
Eventually(func() error {
118+
return applyCronJob(ctx, "test-opaque-short", "abc", "")
119+
}, 5*time.Second, 1*time.Second).Should(Succeed())
120+
121+
By("rejecting nonOpaqueField with length 3 (inherits type-level MinLength=4)")
122+
err := applyCronJob(ctx, "test-non-opaque-short", "", "abc")
123+
Expect(err).To(HaveOccurred())
124+
Expect(err.Error()).To(ContainSubstring("should be at least 4 chars long"))
125+
126+
By("rejecting opaqueField with length 6 (applies field-level MaxLength=5)")
127+
err = applyCronJob(ctx, "test-opaque-long", "abcdef", "")
128+
Expect(err).To(HaveOccurred())
129+
Expect(err.Error()).To(ContainSubstring("Too long"))
130+
131+
By("rejecting nonOpaqueField with length 6 (applies field-level MaxLength=5)")
132+
err = applyCronJob(ctx, "test-non-opaque-long", "", "abcdef")
133+
Expect(err).To(HaveOccurred())
134+
Expect(err.Error()).To(ContainSubstring("Too long"))
135+
})
136+
})
39137
})

‎pkg/crd/testdata/cronjob_types.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,15 @@ type CronJobSpec struct {
430430
// +kubebuilder:validation:MinLength=10
431431
// +kubebuilder:validation:MaxLength=10
432432
FieldLevelLocalDeclarationOverride LongerString `json:"fieldLevelLocalDeclarationOverride,omitempty"`
433+
434+
// This tests that +k8s:opaque suppresses type-level validation from LongerString (MinLength=4).
435+
// +k8s:opaque
436+
// +kubebuilder:validation:MaxLength=5
437+
OpaqueField LongerString `json:"opaqueField,omitempty"`
438+
439+
// This tests that without +k8s:opaque, type-level validations from LongerString are inherited.
440+
// +kubebuilder:validation:MaxLength=5
441+
NonOpaqueField LongerString `json:"nonOpaqueField,omitempty"`
433442
}
434443

435444
type InlineAlias = EmbeddedStruct

‎pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9542,6 +9542,12 @@ spec:
95429542
This flag is like suspend, but for when you really mean it.
95439543
It helps test the +kubebuilder:validation:Type marker.
95449544
type: string
9545+
nonOpaqueField:
9546+
description: This tests that without +k8s:opaque, type-level validations
9547+
from LongerString are inherited.
9548+
maxLength: 5
9549+
minLength: 4
9550+
type: string
95459551
onlyAllowSettingOnUpdate:
95469552
description: Test that we can add a field that can only be set to
95479553
a non-default value on updates using XValidation OptionalOldSelf.
@@ -9552,6 +9558,11 @@ spec:
95529558
an update.
95539559
optionalOldSelf: true
95549560
rule: oldSelf.hasValue() || self == 0
9561+
opaqueField:
9562+
description: This tests that +k8s:opaque suppresses type-level validation
9563+
from LongerString (MinLength=4).
9564+
maxLength: 5
9565+
type: string
95559566
patternObject:
95569567
description: This tests that pattern validator is properly applied.
95579568
pattern: ^$|^((https):\/\/?)[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|\/?))$

0 commit comments

Comments
 (0)