From e65429b1e5990dd019ebb7b5642dcd22a3e9cd13 Mon Sep 17 00:00:00 2001 From: Yann Hamon Date: Mon, 12 May 2025 11:15:53 +0200 Subject: [PATCH] Add support for duration (#328) * Add custom validation logic for durations --- acceptance.bats | 8 +- fixtures/grafana-alert-rule-group-sample.yaml | 62 ++++ fixtures/grafanaalertrulegroup_v1beta1.json | 334 ++++++++++++++++++ pkg/validator/validator.go | 81 +++++ pkg/validator/validator_test.go | 81 +++++ 5 files changed, 565 insertions(+), 1 deletion(-) create mode 100644 fixtures/grafana-alert-rule-group-sample.yaml create mode 100644 fixtures/grafanaalertrulegroup_v1beta1.json diff --git a/acceptance.bats b/acceptance.bats index 97e0ad1..d01e9f2 100755 --- a/acceptance.bats +++ b/acceptance.bats @@ -358,4 +358,10 @@ resetCacheFolder() { @test "passes when trying to use a CRD that does not have the JSONSchema set" { run bash -c "bin/kubeconform -schema-location default -schema-location 'https://raw.githubusercontent.com/datreeio/CRDs-catalog/main/{{.Group}}/{{.ResourceKind}}_{{.ResourceAPIVersion}}.json' fixtures/httpproxy.yaml" [ "$status" -eq 0 ] -} \ No newline at end of file +} + +# https://github.com/yannh/kubeconform/pull/309 +@test "passes when validating duration not in ISO8601" { + run bash -c "./bin/kubeconform -schema-location ./fixtures/grafanaalertrulegroup_v1beta1.json ./fixtures/grafana-alert-rule-group-sample.yaml" + [ "$status" -eq 0 ] +} diff --git a/fixtures/grafana-alert-rule-group-sample.yaml b/fixtures/grafana-alert-rule-group-sample.yaml new file mode 100644 index 0000000..4f11aff --- /dev/null +++ b/fixtures/grafana-alert-rule-group-sample.yaml @@ -0,0 +1,62 @@ +--- +apiVersion: grafana.integreatly.org/v1beta1 +kind: GrafanaAlertRuleGroup +metadata: + name: grafanaalertrulegroup-sample +spec: + folderRef: test-folder + instanceSelector: + matchLabels: + dashboards: "grafana" + interval: 5m + rules: + - condition: B + data: + - datasourceUid: grafanacloud-demoinfra-prom + model: + datasource: + type: prometheus + uid: grafanacloud-demoinfra-prom + editorMode: code + expr: weather_temp_c{} + instant: true + intervalMs: 1000 + legendFormat: __auto + maxDataPoints: 43200 + range: false + refId: A + refId: A + relativeTimeRange: + from: 600 + - datasourceUid: __expr__ + model: + conditions: + - evaluator: + params: + - 0 + type: lt + operator: + type: and + query: + params: + - C + reducer: + params: [] + type: last + type: query + datasource: + type: __expr__ + uid: __expr__ + expression: A + intervalMs: 1000 + maxDataPoints: 43200 + refId: B + type: threshold + refId: B + relativeTimeRange: + from: 600 + execErrState: Error + for: 5m0s + noDataState: NoData + title: Temperature below zero + uid: 4843de5c-4f8a-4af0-9509-23526a04faf8 diff --git a/fixtures/grafanaalertrulegroup_v1beta1.json b/fixtures/grafanaalertrulegroup_v1beta1.json new file mode 100644 index 0000000..27b04d7 --- /dev/null +++ b/fixtures/grafanaalertrulegroup_v1beta1.json @@ -0,0 +1,334 @@ +{ + "description": "GrafanaAlertRuleGroup is the Schema for the grafanaalertrulegroups API", + "properties": { + "apiVersion": { + "description": "APIVersion defines the versioned schema of this representation of an object.\nServers should convert recognized schemas to the latest internal value, and\nmay reject unrecognized values.\nMore info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", + "type": "string" + }, + "kind": { + "description": "Kind is a string value representing the REST resource this object represents.\nServers may infer this from the endpoint the client submits requests to.\nCannot be updated.\nIn CamelCase.\nMore info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", + "type": "string" + }, + "metadata": { + "type": "object" + }, + "spec": { + "description": "GrafanaAlertRuleGroupSpec defines the desired state of GrafanaAlertRuleGroup", + "properties": { + "allowCrossNamespaceImport": { + "type": "boolean" + }, + "editable": { + "description": "Whether to enable or disable editing of the alert rule group in Grafana UI", + "type": "boolean", + "x-kubernetes-validations": [ + { + "message": "Value is immutable", + "rule": "self == oldSelf" + } + ] + }, + "folderRef": { + "description": "Match GrafanaFolders CRs to infer the uid", + "type": "string" + }, + "folderUID": { + "description": "UID of the folder containing this rule group\nOverrides the FolderSelector", + "type": "string" + }, + "instanceSelector": { + "description": "selects Grafanas for import", + "properties": { + "matchExpressions": { + "description": "matchExpressions is a list of label selector requirements. The requirements are ANDed.", + "items": { + "description": "A label selector requirement is a selector that contains values, a key, and an operator that\nrelates the key and values.", + "properties": { + "key": { + "description": "key is the label key that the selector applies to.", + "type": "string" + }, + "operator": { + "description": "operator represents a key's relationship to a set of values.\nValid operators are In, NotIn, Exists and DoesNotExist.", + "type": "string" + }, + "values": { + "description": "values is an array of string values. If the operator is In or NotIn,\nthe values array must be non-empty. If the operator is Exists or DoesNotExist,\nthe values array must be empty. This array is replaced during a strategic\nmerge patch.", + "items": { + "type": "string" + }, + "type": "array", + "x-kubernetes-list-type": "atomic" + } + }, + "required": [ + "key", + "operator" + ], + "type": "object", + "additionalProperties": false + }, + "type": "array", + "x-kubernetes-list-type": "atomic" + }, + "matchLabels": { + "additionalProperties": { + "type": "string" + }, + "description": "matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels\nmap is equivalent to an element of matchExpressions, whose key field is \"key\", the\noperator is \"In\", and the values array contains only \"value\". The requirements are ANDed.", + "type": "object" + } + }, + "type": "object", + "x-kubernetes-map-type": "atomic", + "x-kubernetes-validations": [ + { + "message": "Value is immutable", + "rule": "self == oldSelf" + } + ], + "additionalProperties": false + }, + "interval": { + "format": "duration", + "pattern": "^([0-9]+(\\.[0-9]+)?(ns|us|\u00b5s|ms|s|m|h))+$", + "type": "string" + }, + "name": { + "description": "Name of the alert rule group. If not specified, the resource name will be used.", + "type": "string" + }, + "resyncPeriod": { + "default": "10m", + "format": "duration", + "pattern": "^([0-9]+(\\.[0-9]+)?(ns|us|\u00b5s|ms|s|m|h))+$", + "type": "string" + }, + "rules": { + "items": { + "description": "AlertRule defines a specific rule to be evaluated. It is based on the upstream model with some k8s specific type mappings", + "properties": { + "annotations": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "condition": { + "type": "string" + }, + "data": { + "items": { + "properties": { + "datasourceUid": { + "description": "Grafana data source unique identifier; it should be '__expr__' for a Server Side Expression operation.", + "type": "string" + }, + "model": { + "description": "JSON is the raw JSON query and includes the above properties as well as custom properties.", + "x-kubernetes-preserve-unknown-fields": true + }, + "queryType": { + "description": "QueryType is an optional identifier for the type of query.\nIt can be used to distinguish different types of queries.", + "type": "string" + }, + "refId": { + "description": "RefID is the unique identifier of the query, set by the frontend call.", + "type": "string" + }, + "relativeTimeRange": { + "description": "relative time range", + "properties": { + "from": { + "description": "from", + "format": "int64", + "type": "integer" + }, + "to": { + "description": "to", + "format": "int64", + "type": "integer" + } + }, + "type": "object", + "additionalProperties": false + } + }, + "type": "object", + "additionalProperties": false + }, + "type": "array" + }, + "execErrState": { + "enum": [ + "OK", + "Alerting", + "Error", + "KeepLast" + ], + "type": "string" + }, + "for": { + "format": "duration", + "pattern": "^([0-9]+(\\.[0-9]+)?(ns|us|\u00b5s|ms|s|m|h))+$", + "type": "string" + }, + "isPaused": { + "type": "boolean" + }, + "labels": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "noDataState": { + "enum": [ + "Alerting", + "NoData", + "OK", + "KeepLast" + ], + "type": "string" + }, + "notificationSettings": { + "properties": { + "group_by": { + "items": { + "type": "string" + }, + "type": "array" + }, + "group_interval": { + "type": "string" + }, + "group_wait": { + "type": "string" + }, + "mute_time_intervals": { + "items": { + "type": "string" + }, + "type": "array" + }, + "receiver": { + "type": "string" + }, + "repeat_interval": { + "type": "string" + } + }, + "required": [ + "receiver" + ], + "type": "object", + "additionalProperties": false + }, + "title": { + "example": "Always firing", + "maxLength": 190, + "minLength": 1, + "type": "string" + }, + "uid": { + "pattern": "^[a-zA-Z0-9-_]+$", + "type": "string" + } + }, + "required": [ + "condition", + "data", + "execErrState", + "for", + "noDataState", + "title", + "uid" + ], + "type": "object", + "additionalProperties": false + }, + "type": "array" + } + }, + "required": [ + "instanceSelector", + "interval", + "rules" + ], + "type": "object", + "x-kubernetes-validations": [ + { + "message": "Only one of FolderUID or FolderRef can be set", + "rule": "(has(self.folderUID) && !(has(self.folderRef))) || (has(self.folderRef) && !(has(self.folderUID)))" + } + ], + "additionalProperties": false + }, + "status": { + "description": "GrafanaAlertRuleGroupStatus defines the observed state of GrafanaAlertRuleGroup", + "properties": { + "conditions": { + "items": { + "description": "Condition contains details for one aspect of the current state of this API Resource.", + "properties": { + "lastTransitionTime": { + "description": "lastTransitionTime is the last time the condition transitioned from one status to another.\nThis should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.", + "format": "date-time", + "type": "string" + }, + "message": { + "description": "message is a human readable message indicating details about the transition.\nThis may be an empty string.", + "maxLength": 32768, + "type": "string" + }, + "observedGeneration": { + "description": "observedGeneration represents the .metadata.generation that the condition was set based upon.\nFor instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date\nwith respect to the current state of the instance.", + "format": "int64", + "minimum": 0, + "type": "integer" + }, + "reason": { + "description": "reason contains a programmatic identifier indicating the reason for the condition's last transition.\nProducers of specific condition types may define expected values and meanings for this field,\nand whether the values are considered a guaranteed API.\nThe value should be a CamelCase string.\nThis field may not be empty.", + "maxLength": 1024, + "minLength": 1, + "pattern": "^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$", + "type": "string" + }, + "status": { + "description": "status of the condition, one of True, False, Unknown.", + "enum": [ + "True", + "False", + "Unknown" + ], + "type": "string" + }, + "type": { + "description": "type of condition in CamelCase or in foo.example.com/CamelCase.", + "maxLength": 316, + "pattern": "^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$", + "type": "string" + } + }, + "required": [ + "lastTransitionTime", + "message", + "reason", + "status", + "type" + ], + "type": "object", + "additionalProperties": false + }, + "type": "array" + } + }, + "required": [ + "conditions" + ], + "type": "object", + "additionalProperties": false + } + }, + "type": "object" +} diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index 125bbfc..2412ac8 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -16,6 +16,7 @@ import ( "os" "sigs.k8s.io/yaml" "strings" + "time" ) // Different types of validation results @@ -284,6 +285,85 @@ func (val *v) Validate(filename string, r io.ReadCloser) []Result { return val.ValidateWithContext(context.Background(), filename, r) } +// validateDuration is a custom validator for the duration format +// as JSONSchema only supports the ISO 8601 format, i.e. `PT1H30M`, +// while Kubernetes API machinery expects the Go duration format, i.e. `1h30m` +// which is commonly used in Kubernetes operators for specifying intervals. +// https://github.com/kubernetes/apiextensions-apiserver/blob/1ecd29f74da0639e2e6e3b8fac0c9bfd217e05eb/pkg/apis/apiextensions/v1/types_jsonschema.go#L71 +func validateDuration(v any) error { + // Try validation with the Go duration format + if _, err := time.ParseDuration(v.(string)); err == nil { + return nil + } + + s, ok := v.(string) + if !ok { + return nil + } + + // must start with 'P' + s, ok = strings.CutPrefix(s, "P") + if !ok { + return fmt.Errorf("must start with P") + } + if s == "" { + return fmt.Errorf("nothing after P") + } + + // dur-week + if s, ok := strings.CutSuffix(s, "W"); ok { + if s == "" { + return fmt.Errorf("no number in week") + } + for _, ch := range s { + if ch < '0' || ch > '9' { + return fmt.Errorf("invalid week") + } + } + return nil + } + + allUnits := []string{"YMD", "HMS"} + for i, s := range strings.Split(s, "T") { + if i != 0 && s == "" { + return fmt.Errorf("no time elements") + } + if i >= len(allUnits) { + return fmt.Errorf("more than one T") + } + units := allUnits[i] + for s != "" { + digitCount := 0 + for _, ch := range s { + if ch >= '0' && ch <= '9' { + digitCount++ + } else { + break + } + } + if digitCount == 0 { + return fmt.Errorf("missing number") + } + s = s[digitCount:] + if s == "" { + return fmt.Errorf("missing unit") + } + unit := s[0] + j := strings.IndexByte(units, unit) + if j == -1 { + if strings.IndexByte(allUnits[i], unit) != -1 { + return fmt.Errorf("unit %q out of order", unit) + } + return fmt.Errorf("invalid unit %q", unit) + } + units = units[j+1:] + s = s[1:] + } + } + + return nil +} + func downloadSchema(registries []registry.Registry, l jsonschema.SchemeURLLoader, kind, version, k8sVersion string) (*jsonschema.Schema, error) { var err error var path string @@ -293,6 +373,7 @@ func downloadSchema(registries []registry.Registry, l jsonschema.SchemeURLLoader path, s, err = reg.DownloadSchema(kind, version, k8sVersion) if err == nil { c := jsonschema.NewCompiler() + c.RegisterFormat(&jsonschema.Format{"duration", validateDuration}) c.UseLoader(l) c.DefaultDraft(jsonschema.Draft4) if err := c.AddResource(path, s); err != nil { diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index cc77e6d..36a2b3f 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -379,6 +379,87 @@ lastName: bar Error, []ValidationError{}, }, + { + "valid resource duration - go format", + []byte(` +kind: name +apiVersion: v1 +interval: 5s +`), + []byte(`{ + "title": "Example Schema", + "type": "object", + "properties": { + "kind": { + "type": "string" + }, + "interval": { + "type": "string", + "format": "duration" + } + }, + "required": ["interval"] +}`), + nil, + false, + false, + Valid, + []ValidationError{}, + }, + { + "valid resource duration - iso8601 format", + []byte(` +kind: name +apiVersion: v1 +interval: PT1H +`), + []byte(`{ + "title": "Example Schema", + "type": "object", + "properties": { + "kind": { + "type": "string" + }, + "interval": { + "type": "string", + "format": "duration" + } + }, + "required": ["interval"] +}`), + nil, + false, + false, + Valid, + []ValidationError{}, + }, + { + "invalid resource duration", + []byte(` +kind: name +apiVersion: v1 +interval: test +`), + []byte(`{ + "title": "Example Schema", + "type": "object", + "properties": { + "kind": { + "type": "string" + }, + "interval": { + "type": "string", + "format": "duration" + } + }, + "required": ["interval"] +}`), + nil, + false, + false, + Invalid, + []ValidationError{{Path: "/interval", Msg: "'test' is not valid duration: must start with P"}}, + }, } { val := v{ opts: Opts{