Add regular expression (regex) pattern support

[chlunde]

Is your feature request related to a problem?
When creating a CR, only the documentation and types are available. Sometimes the type is string and the documentation is brief or non-existent, for example for a role of type string. The api-2.json files often contains a validation pattern for this case, but the author of a CR does not have that information available, and will have to create the resource in a cluster and observe the result to check if the format is correct.

Describe the solution you'd like
Consider adding kubebuilder Pattern annotations. In order to do this we would have to expose this information in aws-sdk-for-go v1, that seems trivial:

diff -ruwbp /home/chlunde/go/pkg/mod/github.com/aws/[email protected]/private/model/api/shape.go ./private/model/api/shape.go
--- /home/chlunde/go/pkg/mod/github.com/aws/[email protected]/private/model/api/shape.go	2021-09-05 13:33:33.430030539 +0200
+++ ./private/model/api/shape.go	2021-09-05 21:59:57.401057747 +0200
@@ -80,6 +81,7 @@ type Shape struct {
 	MemberRef        ShapeRef             `json:"member"` // List ref
 	KeyRef           ShapeRef             `json:"key"`    // map key ref
 	ValueRef         ShapeRef             `json:"value"`  // map value ref
+	Pattern          string               `json:"pattern"`
 	Required         []string
 	Payload          string
 	Type             string

Finally we can add it to the template, similar to

diff --git a/templates/crossplane/apis/type_def.go.tpl b/templates/crossplane/apis/type_def.go.tpl
index 6a0cbdc..4598d98 100644
--- a/templates/crossplane/apis/type_def.go.tpl
+++ b/templates/crossplane/apis/type_def.go.tpl
@@ -4,6 +4,9 @@ type {{ .Names.Camel }} struct {
        {{- if $attr.Shape }}
        {{ $attr.Shape.Documentation }}
        {{- end }}
+       {{- if $attr.Shape.Pattern -}}
+       // +kubebuilder:validation:Pattern:={{ $attr.Shape.Pattern }}
+       {{- end }}
        {{ $attr.Names.Camel }} {{ $attr.GoType }} `json:"{{ $attr.Names.CamelLower }},omitempty"`
 {{- end }}
 }

The result

• richer validation
• documentation
• possibility of detecting errors using static checking early in gitops pipelines and development environments
• detecting errors before sending to the AWS API (if there is no static checking set up/not gitops)

Example (I have not tested this in a cluster, so the syntax might not be exactly right):

+       // +kubebuilder:validation:Pattern:=arn:(aws[a-zA-Z-]*)?:iam::\d{12}:role/?[a-zA-Z_0-9+=,.@\-_/]+
        Role *string `json:"role,omitempty"`
+       // +kubebuilder:validation:Pattern:=arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\-])+:([a-z]{2}(-gov)?-[a-z]+-\d{1})?:(\d{12})?:(.*)
        SigningJobARN *string `json:"signingJobARN,omitempty"`
+       // +kubebuilder:validation:Pattern:=arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\-])+:([a-z]{2}(-gov)?-[a-z]+-\d{1})?:(\d{12})?:(.*)
        SigningProfileVersionARN *string `json:"signingProfileVersionARN,omitempty"`
+       // +kubebuilder:validation:Pattern:=(\$LATEST|[0-9]+)
        Version *string `json:"version,omitempty"`

Describe alternatives you've considered
Keeping it as it is? 🤔

Things to consider
We would also have to understand what possible differences between the regex language in the JSON files are, and what the API server validates, and possibly convert (or drop) some patterns. We should also consider if introducing validations will cause other problems, and possibly an escape hatch in generator-config.yaml to ignore the pattern for a specific attribute?

Related issue: crossplane-contrib/provider-aws#572

cc @muvaf

[jaypipes]

Hi @chlunde! This is a great feature request and something I think we should be able to accommodate fairly easily. I'll see what I can do about getting this into our next sprint or two.

[ack-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

[a-hilaly]

/lifecycle frozen

[knottnt]

One concern I have with this feature request is that with stricter validation of field patterns we lose flexibility when/if an underlying AWS API changes its pattern or a mismatch is introduced some other way. If an ACK CRD and underlying AWS API’s pattern became out of sync we’d need to re-generate the ACK controller and publish a new version before users could resolve the mismatch. Note, this doesn’t necessarily require a breaking change on the AWS API side as the new pattern could be a superset of the old pattern.

We could update our API Reference docs and CRD descriptions to include this information with less potential downside in the event of a mismatch. Would that help address some of your concerns?

[knottnt]

/remove-lifecycle frozen