Skip to content

Commit

Permalink
Merge pull request #255 from dalton-hill-0/escape-cel-reserved-keywords
Browse files Browse the repository at this point in the history 
Escape CEL Reserved Keywords when Generating Validation Rules
  • Loading branch information
@ulucinar
ulucinar authored Aug 21, 2023
2 parents 1a41e99 + 322c5eb commit e620c62
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 7 deletions.
24 changes: 22 additions & 2 deletions pkg/types/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ const (
wildcard = "*"

emptyStruct = "struct{}"

// ref: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules
celEscapeSequence = "__%s__"
)

var (
// ref: https://github.com/google/cel-spec/blob/v0.6.0/doc/langdef.md#syntax
celReservedKeywords = []string{"true", "false", "null", "in", "as", "break", "const", "continue",
"else", "for", "function", "if", "import", "let", "loop", "package", "namespace", "return", "var",
"void", "while"}
)

// Generated is a struct that holds generated types
Expand Down Expand Up @@ -139,10 +149,11 @@ func (g *Builder) AddToBuilder(typeNames *TypeNames, r *resource) (*types.Named,

for _, p := range r.topLevelRequiredParams {
g.validationRules += "\n"
sp := sanitizePath(p.path)
if p.includeInit {
g.validationRules += fmt.Sprintf(`// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.%s) || has(self.initProvider.%s)",message="%s is a required parameter"`, p.path, p.path, p.path)
g.validationRules += fmt.Sprintf(`// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.%s) || has(self.initProvider.%s)",message="%s is a required parameter"`, sp, sp, p.path)
} else {
g.validationRules += fmt.Sprintf(`// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.%s)",message="%s is a required parameter"`, p.path, p.path)
g.validationRules += fmt.Sprintf(`// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.%s)",message="%s is a required parameter"`, sp, p.path)
}
}

Expand Down Expand Up @@ -445,3 +456,12 @@ func fieldPathWithWildcard(parts []string) string {
}
return seg.String()
}

func sanitizePath(p string) string {
for _, reserved := range celReservedKeywords {
if p == reserved {
return fmt.Sprintf(celEscapeSequence, p)
}
}
return p
}
52 changes: 47 additions & 5 deletions pkg/types/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,10 @@ func TestBuild(t *testing.T) {
cfg *config.Resource
}
type want struct {
forProvider string
atProvider string
err error
forProvider string
atProvider string
validationRules string
err error
}
cases := map[string]struct {
args
Expand Down Expand Up @@ -251,6 +252,9 @@ func TestBuild(t *testing.T) {
want: want{
forProvider: `type example.Parameters struct{Enable *bool "json:\"enable,omitempty\" tf:\"enable,omitempty\""; ID *int64 "json:\"id,omitempty\" tf:\"id,omitempty\""; Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""}`,
atProvider: `type example.Observation struct{Config *string "json:\"config,omitempty\" tf:\"config,omitempty\""; Enable *bool "json:\"enable,omitempty\" tf:\"enable,omitempty\""; ID *int64 "json:\"id,omitempty\" tf:\"id,omitempty\""; Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""; Value *float64 "json:\"value,omitempty\" tf:\"value,omitempty\""}`,
validationRules: `
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.id) || has(self.initProvider.id)",message="id is a required parameter"
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.name) || has(self.initProvider.name)",message="name is a required parameter"`,
},
},
"Resource_Types": {
Expand Down Expand Up @@ -284,6 +288,9 @@ func TestBuild(t *testing.T) {
want: want{
forProvider: `type example.Parameters struct{List []*string "json:\"list,omitempty\" tf:\"list,omitempty\""; ResourceIn map[string]example.ResourceInParameters "json:\"resourceIn,omitempty\" tf:\"resource_in,omitempty\""}`,
atProvider: `type example.Observation struct{List []*string "json:\"list,omitempty\" tf:\"list,omitempty\""; ResourceIn map[string]example.ResourceInParameters "json:\"resourceIn,omitempty\" tf:\"resource_in,omitempty\""; ResourceOut map[string]example.ResourceOutObservation "json:\"resourceOut,omitempty\" tf:\"resource_out,omitempty\""}`,
validationRules: `
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.list) || has(self.initProvider.list)",message="list is a required parameter"
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.resourceIn) || has(self.initProvider.resourceIn)",message="resourceIn is a required parameter"`,
},
},
"Sensitive_Fields": {
Expand Down Expand Up @@ -311,6 +318,9 @@ func TestBuild(t *testing.T) {
want: want{
forProvider: `type example.Parameters struct{Key1SecretRef *github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector "json:\"key1SecretRef,omitempty\" tf:\"-\""; Key2SecretRef github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector "json:\"key2SecretRef\" tf:\"-\""; Key3SecretRef []github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector "json:\"key3SecretRef\" tf:\"-\""}`,
atProvider: `type example.Observation struct{}`,
validationRules: `
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.key2SecretRef)",message="key2SecretRef is a required parameter"
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.key3SecretRef)",message="key3SecretRef is a required parameter"`,
},
},
"Invalid_Sensitive_Fields": {
Expand Down Expand Up @@ -356,6 +366,8 @@ func TestBuild(t *testing.T) {
want: want{
forProvider: `type example.Parameters struct{Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""; ReferenceID *string "json:\"referenceId,omitempty\" tf:\"reference_id,omitempty\""; ExternalResourceID *github.com/crossplane/crossplane-runtime/apis/common/v1.Reference "json:\"externalResourceId,omitempty\" tf:\"-\""; ReferenceIDSelector *github.com/crossplane/crossplane-runtime/apis/common/v1.Selector "json:\"referenceIdSelector,omitempty\" tf:\"-\""}`,
atProvider: `type example.Observation struct{Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""; ReferenceID *string "json:\"referenceId,omitempty\" tf:\"reference_id,omitempty\""}`,
validationRules: `
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.name) || has(self.initProvider.name)",message="name is a required parameter"`,
},
},
"Invalid_Schema_Type": {
Expand All @@ -375,6 +387,33 @@ func TestBuild(t *testing.T) {
err: errors.Wrapf(errors.Wrapf(errors.Errorf("invalid schema type %s", "TypeInvalid"), "cannot infer type from schema of field %s", "name"), "cannot build the Types"),
},
},
"Validation_Rules_With_Keywords": {
args: args{
cfg: &config.Resource{
TerraformResource: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
},
// "namespace" is a cel reserved value and should be wrapped when used in
// validation rules (i.e., __namespace__)
"namespace": {
Type: schema.TypeString,
Required: true,
},
},
},
},
},
want: want{
forProvider: `type example.Parameters struct{Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""; Namespace *string "json:\"namespace,omitempty\" tf:\"namespace,omitempty\""}`,
atProvider: `type example.Observation struct{Name *string "json:\"name,omitempty\" tf:\"name,omitempty\""; Namespace *string "json:\"namespace,omitempty\" tf:\"namespace,omitempty\""}`,
validationRules: `
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.name) || has(self.initProvider.name)",message="name is a required parameter"
// +kubebuilder:validation:XValidation:rule="!('*' in self.managementPolicies || 'Create' in self.managementPolicies || 'Update' in self.managementPolicies) || has(self.forProvider.__namespace__) || has(self.initProvider.__namespace__)",message="namespace is a required parameter"`,
},
},
}
for n, tc := range cases {
t.Run(n, func(t *testing.T) {
Expand All @@ -385,15 +424,18 @@ func TestBuild(t *testing.T) {
t.Fatalf("Build(...): -want error, +got error: %s", diff)
}
if g.ForProviderType != nil {
if diff := cmp.Diff(tc.want.forProvider, g.ForProviderType.Obj().String(), test.EquateErrors()); diff != "" {
if diff := cmp.Diff(tc.want.forProvider, g.ForProviderType.Obj().String()); diff != "" {
t.Fatalf("Build(...): -want forProvider, +got forProvider: %s", diff)
}
}
if g.AtProviderType != nil {
if diff := cmp.Diff(tc.want.atProvider, g.AtProviderType.Obj().String(), test.EquateErrors()); diff != "" {
if diff := cmp.Diff(tc.want.atProvider, g.AtProviderType.Obj().String()); diff != "" {
t.Fatalf("Build(...): -want atProvider, +got atProvider: %s", diff)
}
}
if diff := cmp.Diff(tc.want.validationRules, g.ValidationRules); diff != "" {
t.Fatalf("Build(...): -want validationRules, +got validationRules: %s", diff)
}
})
}
}

0 comments on commit e620c62

Please sign in to comment.