Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add marker comment support for annotating cluster scopedness #470

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
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
Loading