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

feat: CEL validation of valuesFrom object ensuring refs are defined #1729

Conversation

Baarsgaard
Copy link
Contributor

@Baarsgaard Baarsgaard commented Oct 27, 2024

The valuesFrom[*].valuefrom field on GrafanaContactPoint can be an empty {} resulting in nil errors in the log when reconciling.

I personally ran into this when I missed a capital K in secretKeyRef by mistake and Kubernetes dropped the field once applied.

The following test resources are perfectly valid but the last GrafanaContactPoint will trigger a panic.

Expand...
2024-10-27T11:53:13Z    ERROR   Observed a panic        {"controller": "grafanacontactpoint", "controllerGroup": "grafana.integreatly.org", "controllerKind": "GrafanaContactPoint", "GrafanaContactPoint": {"name":"invalid-ref","namespace":"default"}, "namespace": "default", "name": "invalid-ref", "reconcileID": "c2b210b4-9b41-485c-8182-6c74fd1e8371", 
  "panic": "runtime error: invalid memory address or nil pointer dereference", "panicGoValue": "\"invalid memory address or nil pointer dereference\"", "stacktrace": "goroutine 342 [running]:\nk8s.io/apimachinery/pkg/util/runtime.logPanic({0x23d5f78, 0xc000d09770}, {0x1d1bb40, 0x3630450})\n\tk8s.io/[email protected]/pkg/util/runtime/runtime.go:107 
  +0xbc\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile.func1()\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:105
   +0x114\npanic({0x1d1bb40?, 0x3630450?})\n\truntime/panic.go:770
   +0x132\ngithub.com/grafana/grafana-operator/v5/controllers.getReferencedValue({0x23d5f78, 0xc000d09770}, {0x23e1960, 0xc0001566c0}, {0x23abfc0?, 0xc000d96d80?}, {0x0?, 0x0?})\n\tgithub.com/grafana/grafana-operator/v5/controllers/controller_shared.go:250
    +0x25a\ngithub.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).buildSettings(0xc000574690, {0x23d5f78, 0xc000d09770}, 0xc000d96d80)\n\tgithub.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:227
     +0x285\ngithub.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).reconcileWithInstance(0xc000574690, {0x23d5f78, 0xc000d09770}, 0xc000b2c008, 0xc000d96d80)\n\tgithub.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:184
  +0x1e9\ngithub.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).Reconcile(0xc000574690, {0x23d5f78, 0xc000d09770}, {{{0xc00092d160?, 0x2054d65?}, {0xc00092d150?, 0x100?}}})\n\tgithub.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:138
    +0xc72\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile(0xc000d096e0?, {0x23d5f78?, 0xc000d09770?}, {{{0xc00092d160?, 0x0?}, {0xc00092d150?, 0x0?}}})\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:116
     +0xd4\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).reconcileHandler(0x23e5aa0, {0x23d5fb0, 0xc0006d2af0}, {{{0xc00092d160, 0x7}, {0xc00092d150, 0xb}}})\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:303
      +0x3b8\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).processNextWorkItem(0x23e5aa0, {0x23d5fb0, 0xc0006d2af0})\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:263
      +0x21d\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2.2()\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:224
      +0x8a\ncreated by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2 in goroutine 224\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:220 +0x490\n"}
runtime.sigpanic
        runtime/signal_unix.go:881
github.com/grafana/grafana-operator/v5/controllers.getReferencedValue
        github.com/grafana/grafana-operator/v5/controllers/controller_shared.go:250
github.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).buildSettings
        github.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:227
github.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).reconcileWithInstance
        github.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:184
github.com/grafana/grafana-operator/v5/controllers.(*GrafanaContactPointReconciler).Reconcile
        github.com/grafana/grafana-operator/v5/controllers/grafanacontactpoint_controller.go:138
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile
        sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:116
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).reconcileHandler
        sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:303
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).processNextWorkItem
        sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:263
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2.2
        sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:224

With this change, a validation error will be presented instead:
The GrafanaContactPoint "invalid-ref" is invalid: spec.valuesFrom[0].valueFrom: Invalid value: "object": Either configMapKeyRef or secretKeyRef must be set

Test resources:

apiVersion: v1
kind: ConfigMap
metadata:
  name: test
data:
  test: test
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaContactPoint
metadata:
  name: normal
  namespace: default
spec:
  instanceSelector:
    matchLabels:
      dashboards: grafana
  name: normal
  settings:
    addresses: [email protected]
  type: email
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaContactPoint
metadata:
  name: normal-with-ref
  namespace: default
spec:
  instanceSelector:
    matchLabels:
      dashboards: grafana
  name: with-ref
  type: email
  settings: {}
  valuesFrom:
    - targetPath: addresses
      valueFrom:
        configMapKeyRef:
          name: test
          key: test
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaContactPoint
metadata:
  name: invalid-ref
  namespace: default
spec:
  instanceSelector:
    matchLabels:
      dashboards: grafana
  name: invalid-ref
  type: email
  settings: {}
  valuesFrom:
    - targetPath: addresses
      valueFrom: {}
The reason for maxItems marker

I ran into the following test error without it:

2024-10-27T12:39:56+01:00       DEBUG   controller-runtime.test-env     installing CRD  {"crd": "grafanacontactpoints.grafana.integreatly.org"}
Failure [2.613 seconds]
[BeforeSuite] BeforeSuite
/home/ste/grafana-operator/controllers/suite_test.go:51

  Unexpected error:
      <*fmt.wrapError | 0xc0006902e0>:
      unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD "grafanacontactpoints.grafana.integreatly.org": CustomResourceDefinition.apiextensions.k8s.io "grafanacontactpoints.grafana.integreatly.org" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)
      {
          msg: "unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD \"grafanacontactpoints.grafana.integreatly.org\": CustomResourceDefinition.apiextensions.k8s.io \"grafanacontactpoints.grafana.integreatly.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)",
          err: <*fmt.wrapError | 0xc0006902c0>{
              msg: "unable to create CRD instances: unable to create CRD \"grafanacontactpoints.grafana.integreatly.org\": CustomResourceDefinition.apiextensions.k8s.io \"grafanacontactpoints.grafana.integreatly.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)",
              err: <*fmt.wrapError | 0xc0006902a0>{
                  msg: "unable to create CRD \"grafanacontactpoints.grafana.integreatly.org\": CustomResourceDefinition.apiextensions.k8s.io \"grafanacontactpoints.grafana.integreatly.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)",
                  err: <*errors.StatusError | 0xc000244820>{
                      ErrStatus: {
                          TypeMeta: {Kind: "", APIVersion: ""},
                          ListMeta: {
                              SelfLink: "",
                              ResourceVersion: "",
                              Continue: "",
                              RemainingItemCount: nil,
                          },
                          Status: "Failure",
                          Message: "CustomResourceDefinition.apiextensions.k8s.io \"grafanacontactpoints.grafana.integreatly.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule: Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)",
                          Reason: "Invalid",
                          Details: {
                              Name: "grafanacontactpoints.grafana.integreatly.org",
                              Group: "apiextensions.k8s.io",
                              Kind: "CustomResourceDefinition",
                              UID: "",
                              Causes: [
                                  {
                                      Type: "FieldValueForbidden",
                                      Message: "Forbidden: estimated rule cost exceeds budget by factor of 1.048576x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)",
                                      Field: "spec.validation.openAPIV3Schema.properties[spec].properties[valuesFrom].items.properties[valueFrom].x-kubernetes-validations[0].rule",
                                  },
                              ],
                              RetryAfterSeconds: 0,
                          },
                          Code: 422,
                      },
                  },
              },
          },
      }
  occurred

  controllers/suite_test.go:61

Copy link
Member

@theSuess theSuess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks for the PR

@theSuess theSuess added this pull request to the merge queue Oct 28, 2024
Merged via the queue into grafana:master with commit 15909e8 Oct 28, 2024
14 checks passed
@Baarsgaard Baarsgaard deleted the Prevent_nil_pointer_errors_from_invalid_key_refs branch October 28, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants