solo-io / gloo

The Feature-rich, Kubernetes-native, Next-Generation API Gateway Built on Envoy
https://docs.solo.io/
Apache License 2.0
4.07k stars 437 forks source link

AuthConfig CR validation allows configs to be null #5636

Open antonioberben opened 2 years ago

antonioberben commented 2 years ago

Describe the bug Applying an AuthConfig with configs: null as attribute make gloo to crash. The CRD validation should not allow this scenario.

To Reproduce

  1. Deploy gloo (tested with 1.7.7 and 1.9.3)
  2. Apply following resource:
apiVersion: enterprise.gloo.solo.io/v1
kind: AuthConfig
metadata:
  labels:
    argocd.argoproj.io/instance: agt-backend
  name: agt-backend-auth-plugin
  namespace: gloo-system
spec:
  configs: null
  1. glooctl check shows the error then later.

Expected behavior When the user applies the resource, the validation throw an error.

The solution is to add required in the AuthConfig CRD field to the attribute as the picture shows: config-crd

Additional context

chrisgaun commented 2 years ago

Added this to the Webhook validation Epic

Bslabe123 commented 2 years ago

https://solo-io-corp.slack.com/archives/G01EERAK3KJ/p1656599211406749, https://solo-io-corp.slack.com/archives/G01EERAK3KJ/p1656687013416259, There already exists a nil check in the validation loop here projects/gloo/pkg/plugins/extauth/validate.go:38 but no errors seem to be reported when adding an authconfig with a nil config, a better solution might be to do webhook validation which would mean a rework of the OpenApi schema generation to be able to handle 'required' fields which I believe are not currently supported in proto3.

github-actions[bot] commented 3 months ago

This issue has been marked as stale because of no activity in the last 180 days. It will be closed in the next 180 days unless it is tagged "no stalebot" or other activity occurs.