Skip to content

Commit

Permalink
add marker comment support for annotating cluster scopedness
Browse files Browse the repository at this point in the history
  • Loading branch information
alexzielenski committed Apr 29, 2024
1 parent dc4e619 commit 120d97c
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 9 deletions.
76 changes: 68 additions & 8 deletions pkg/generators/markers.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ func (c *CELTag) Validate() error {
return nil
}

type KindScope string

var (
KindScopeNamespaced KindScope = "Namespaced"
KindScopeCluster KindScope = "Cluster"
)

// commentTags represents the parsed comment tags for a given type. These types are then used to generate schema validations.
// These only include the newer prefixed tags. The older tags are still supported,
// but are not included in this struct. Comment Tags are transformed into a
Expand All @@ -77,7 +84,8 @@ func (c *CELTag) Validate() error {
type commentTags struct {
spec.SchemaProps

CEL []CELTag `json:"cel,omitempty"`
CEL []CELTag `json:"cel,omitempty"`
Scope *KindScope `json:"scope,omitempty"`

// Future markers can all be parsed into this centralized struct...
// Optional bool `json:"optional,omitempty"`
Expand All @@ -91,9 +99,32 @@ func (c commentTags) ValidationSchema() (*spec.Schema, error) {
SchemaProps: c.SchemaProps,
}

if len(c.CEL) > 0 {
celRules := c.CEL

if c.Scope != nil {
switch *c.Scope {
case KindScopeCluster:
celRules = append(celRules, CELTag{
Rule: "!has(self.metadata.__namespace__) || self.metadata.__namespace__.size() == 0",
Message: "not allowed on this type",
Reason: "FieldValueForbidden",
FieldPath: ".metadata.namespace",
})
case KindScopeNamespaced:
celRules = append(celRules, CELTag{
Rule: "has(self.metadata.__namespace__) && self.metadata.__namespace__.size() > 0",
Message: "",
Reason: "FieldValueRequired",
FieldPath: ".metadata.namespace",
})
default:
return nil, fmt.Errorf("invalid scope %q", *c.Scope)
}
}

if len(celRules) > 0 {
// Convert the CELTag to a map[string]interface{} via JSON
celTagJSON, err := json.Marshal(c.CEL)
celTagJSON, err := json.Marshal(celRules)
if err != nil {
return nil, fmt.Errorf("failed to marshal CEL tag: %w", err)
}
Expand Down Expand Up @@ -164,6 +195,10 @@ func (c commentTags) Validate() error {
err = errors.Join(err, fmt.Errorf("invalid CEL tag at index %d: %w", i, celError))
}

if c.Scope != nil && *c.Scope != KindScopeNamespaced && *c.Scope != KindScopeCluster {
err = errors.Join(err, fmt.Errorf("invalid scope %q", *c.Scope))
}

return err
}

Expand Down Expand Up @@ -252,23 +287,48 @@ func ParseCommentTags(t *types.Type, comments []string, prefix string) (*spec.Sc
return nil, fmt.Errorf("failed to marshal marker comments: %w", err)
}

var commentTags commentTags
if err = json.Unmarshal(out, &commentTags); err != nil {
var parsed commentTags
if err = json.Unmarshal(out, &parsed); err != nil {
return nil, fmt.Errorf("failed to unmarshal marker comments: %w", err)
}

// isEmpty := reflect.DeepEqual(parsed, commentTags{})

// Take some defaults from non k8s:validation prefixed tags
// Only take defaults from non k8s:validation tags if there are other
// k8s:validation tags present on the type. This is to prevent a lot of ripple
// effects while this feature is still being tried out
if parsed.Scope == nil {
hasGenClient := false
hasGenClientNonNamespaced := false
for _, c := range comments {
c := strings.TrimSpace(c)
if c == "+genclient" {
hasGenClient = true
} else if c == "+genclient:nonNamespaced" {
hasGenClientNonNamespaced = true
}
}

if hasGenClientNonNamespaced {
parsed.Scope = &KindScopeCluster
} else if hasGenClient {
parsed.Scope = &KindScopeNamespaced
}
}

// Validate the parsed comment tags
validationErrors := commentTags.Validate()
validationErrors := parsed.Validate()

if t != nil {
validationErrors = errors.Join(validationErrors, commentTags.ValidateType(t))
validationErrors = errors.Join(validationErrors, parsed.ValidateType(t))
}

if validationErrors != nil {
return nil, fmt.Errorf("invalid marker comments: %w", validationErrors)
}

return commentTags.ValidationSchema()
return parsed.ValidationSchema()
}

var (
Expand Down
39 changes: 39 additions & 0 deletions pkg/generators/markers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,45 @@ func TestParseCommentTags(t *testing.T) {
},
expectedError: `failed to parse marker comments: concatenations to key 'cel[0]:message' must be consecutive with its assignment`,
},
{
name: "namespaced scope",
t: structKind,
comments: []string{
`+k8s:validation:scope>Namespaced`,
},
expected: &spec.Schema{
VendorExtensible: spec.VendorExtensible{
Extensions: map[string]interface{}{
"x-kubernetes-validations": []interface{}{
map[string]interface{}{
"rule": "self.metadata.namespace.size() > 0",
"reason": "FieldValueRequired",
},
},
},
},
},
},
{
name: "cluster scope",
t: structKind,
comments: []string{
`+k8s:validation:scope>Cluster`,
},
expected: &spec.Schema{
VendorExtensible: spec.VendorExtensible{
Extensions: map[string]interface{}{
"x-kubernetes-validations": []interface{}{
map[string]interface{}{
"rule": "self.metadata.namespace.size() == 0",
"message": "not allowed on this type",
"reason": "FieldValueForbidden",
},
},
},
},
},
},
}

for _, tc := range cases {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/pkg/generated/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions test/integration/testdata/golden.v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,11 @@
{
"message": "foo",
"rule": "self == oldSelf"
},
{
"fieldPath": ".metadata.namespace",
"reason": "FieldValueRequired",
"rule": "has(self.metadata.__namespace__) \u0026\u0026 self.metadata.__namespace__.size() \u003e 0"
}
]
},
Expand Down
5 changes: 5 additions & 0 deletions test/integration/testdata/golden.v3.json
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,11 @@
{
"message": "foo",
"rule": "self == oldSelf"
},
{
"fieldPath": ".metadata.namespace",
"reason": "FieldValueRequired",
"rule": "has(self.metadata.__namespace__) \u0026\u0026 self.metadata.__namespace__.size() \u003e 0"
}
]
},
Expand Down
1 change: 1 addition & 0 deletions test/integration/testdata/valuevalidation/alpha.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
// +k8s:openapi-gen=true
// +k8s:validation:cel[0]:rule="self == oldSelf"
// +k8s:validation:cel[0]:message="foo"
// +k8s:validation:scope>Namespaced
type Foo struct {
// +k8s:validation:maxLength=5
// +k8s:validation:minLength=1
Expand Down

0 comments on commit 120d97c

Please sign in to comment.