Console Google SSO support in operator #6282

pvsune · 2022-08-31T10:31:11Z

Cover letter

Support Enterprise Google SSO in Console controller as described here.

Backport Required

UX changes

This PR introduces Enterprise fields configurable in the Console CR. Note that Enterprise fields are ignored by Console app if License is not set:

apiVersion: redpanda.vectorized.io/v1alpha1
kind: Console
metadata:
  name: test-console
  namespace: default
spec:
  ...
  login:
    enabled: true
    jwtSecretRef:
      name: "test-console-jwt"
      namespace: "default"
      key: "jwt" # jwt is default if not provided 
    google:
      enabled: true
      clientCredentialsRef:
        name: "test-console-google" # expects clientId, clientSecret in configmap key
        namespace: "default"
      directory:
        serviceAccountRef:
          name: test-console-google-sa # expects sa.json in configmap key
        targetPrincipal: [email protected]
  licenseRef:
    name: "test-console-license"
    namespace: "default"
    key: "license" # license is default if not provided 
  enterprise:
    rbac:
      enabled: true
      roleBindingsRef:
        name: "test-console-rbac" # expects rbac.yaml in configmap key

By design, referenced resources (i.e. ConfigMap Secret) are not watched because they are not owned by the controller. That means updating them requires a manual reload of the Console app (e.g. changes to RBAC file)

Release notes

Features

Introduces NamespaceNameRef type instead of using the full glory of corev1.ObjectReference
Introduce SecretKeyRef type to reference Secret objects
Add defaulting webhook for Console

Improvements

Use Redpanda wildcard certificate for Ingress since Console is exposed through https://console.<rp-subdomain>
Rename ClusterKeyRef to ClusterRef
Run Console kuttl tests in "console" namespace. I think there's no way to get the autogenerated kuttl namespace and we need to refer that in ClusterRef

RafalKorepta

Good Job Left comments.

RafalKorepta · 2022-09-02T08:03:58Z

src/go/k8s/apis/redpanda/v1alpha1/common_types.go

@@ -0,0 +1,43 @@
+package v1alpha1


NIT: commit message shouldn't have dot at the end.

wrong

operator: Support Google SSO Enterprise feature.

good

operator: Support Google SSO Enterprise feature

commit lacks of more context like cover letter have. It could be easier for future ppl to read commit message instead of going to github to read the context.

👍 I updated the commits

RafalKorepta · 2022-09-02T08:22:32Z

src/go/k8s/apis/redpanda/v1alpha1/common_types.go

+// SecretKeyRef contains enough information to inspect or modify the referred Secret data
+// REF https://pkg.go.dev/k8s.io/api/core/v1#ObjectReference
+type SecretKeyRef struct {
+ Name string `json:"name"`
+ Namespace string `json:"namespace"`
+
+ // +optional
+ // Key in Secret data to get value from
+ Key string `json:"key,omitempty"`
+}


Could this be more generic and works for config map too?

We can. But I'm thinking if it is better to have a static mount path for files referenced by configmaps (e.g. Google SA file will always be in /etc/console/enterprise/google/sa.json)

mount path is different thing from key path from configmap. As far as I saw Key is used to point the key inside secret.

I suppose we can do that. Although it feels a little off 😅 that the key is different from the mounted path. We already have validating/defaulting webhook for this though. If possible I'd like to keep this change simple 🙏

src/go/k8s/apis/redpanda/v1alpha1/console_enterprise_types.go

RafalKorepta · 2022-09-02T08:56:23Z

src/go/k8s/pkg/console/configmap.go

+ loginConfig, err := cm.genLogin(ctx)
+ if err != nil {
+ return "", err
+ }
+ if loginConfig != nil {
+ consoleConfig.Login = *loginConfig
+ }


The genLogin could return the real type, so that you would not need to support second if statement. I would return real type not pointer from genLogin

Suggested change

loginConfig, err := cm.genLogin(ctx)

if err != nil {

return "", err

}

if loginConfig != nil {

consoleConfig.Login = *loginConfig

}

consoleConfig.Login, err := cm.genLogin(ctx)

if err != nil {

return "", err

}

src/go/k8s/pkg/console/configmap.go

src/go/k8s/apis/redpanda/v1alpha1/console_enterprise_types.go

RafalKorepta · 2022-09-02T09:02:11Z

src/go/k8s/apis/redpanda/v1alpha1/common_types.go

+ Name string `json:"name"`
+ Namespace string `json:"namespace"`
+


Can you add godoc?

Key field has comment. This is pointed to a newline, do you mean for Namespace, Name?

No this comment points on lines 14-16 where Name and Namespace is not documented in openAPI spec when CRD is generated.

👍 Updated

RafalKorepta

I'm not sure if webhook is correctly implemented as it reverse the order how webhooks should be called

src/go/k8s/pkg/resources/ingress.go

src/go/k8s/config/webhook/manifests.yaml

src/go/k8s/pkg/console/configmap.go

src/go/k8s/webhooks/redpanda/console_webhook.go

src/go/k8s/controllers/redpanda/console_controller_test.go

Support adding SSO in Console using Google as provider This feature requires license and is specified in the Console spec as a SecretKeyRef REF https://docs.redpanda.com/docs/console/single-sign-on/identity-providers/google/

Creating a new certificate from cert-manager takes a long time and sometimes fail Use existing Cluster certificate that has wildcard SANs since we are using "console.<rp-subdomain>"

- Add new mutating webhook that defaults fields for Console - Refactor required keys used for validating/mutating to vars

…jectReference - Rename ClusterKeyRef to ClusterRef - Use the NamespaceNameRef for ClusterRef, Google SSO ClientCredentialsRef - REF https://pkg.go.dev/k8s.io/api/core/v1#ObjectReference

…ttl tests. Default namespace doesn't get cleaned up on succeeding tests But no way to reference the auto-generated kuttl namespace in Console clusterKeyRef spec

github-actions bot added the area/k8s label Aug 31, 2022

pvsune changed the base branch from dev to pvsune/rp-console September 1, 2022 05:18

pvsune force-pushed the enterprise/pvsune/rp-console branch from 5ea26e4 to b1064da Compare September 1, 2022 05:20

pvsune changed the base branch from pvsune/rp-console to dev September 1, 2022 05:20

pvsune force-pushed the enterprise/pvsune/rp-console branch from b1064da to a2a5d19 Compare September 1, 2022 05:21

pvsune changed the title ~~Enterprise/pvsune/rp console~~ Console Google SSO support in operator Sep 1, 2022

pvsune force-pushed the enterprise/pvsune/rp-console branch 6 times, most recently from 5e83353 to 41fc799 Compare September 2, 2022 03:33

pvsune marked this pull request as ready for review September 2, 2022 07:53

pvsune requested a review from a team as a code owner September 2, 2022 07:53

pvsune requested review from alenkacz, joejulian, nicolaferraro, RafalKorepta and vladoschreiner September 2, 2022 07:54

RafalKorepta reviewed Sep 2, 2022

View reviewed changes

pvsune force-pushed the enterprise/pvsune/rp-console branch 5 times, most recently from c06ec16 to 61e7828 Compare September 5, 2022 12:32

RafalKorepta reviewed Sep 5, 2022

View reviewed changes

pvsune force-pushed the enterprise/pvsune/rp-console branch from 61e7828 to 5f06b0c Compare September 8, 2022 00:38

github-actions bot added the area/redpanda label Sep 8, 2022

operator: Support Google SSO Enterprise feature

57e12e3

Support adding SSO in Console using Google as provider This feature requires license and is specified in the Console spec as a SecretKeyRef REF https://docs.redpanda.com/docs/console/single-sign-on/identity-providers/google/

pvsune force-pushed the enterprise/pvsune/rp-console branch from 5f06b0c to a827be5 Compare September 8, 2022 00:42

github-actions bot removed the area/redpanda label Sep 8, 2022

pvsune force-pushed the enterprise/pvsune/rp-console branch 3 times, most recently from fbed6e7 to 3860214 Compare September 8, 2022 06:10

pvsune added 4 commits September 8, 2022 14:20

operator: Use Cluster wildcard certificate in Console Ingress

130d061

Creating a new certificate from cert-manager takes a long time and sometimes fail Use existing Cluster certificate that has wildcard SANs since we are using "console.<rp-subdomain>"

operator: Add defaulting webhook for Enterprise fields

fabd0e7

- Add new mutating webhook that defaults fields for Console - Refactor required keys used for validating/mutating to vars

operator: Introduce NamespaceNameRef type which is used instead of Ob…

4f5f5a4

…jectReference - Rename ClusterKeyRef to ClusterRef - Use the NamespaceNameRef for ClusterRef, Google SSO ClientCredentialsRef - REF https://pkg.go.dev/k8s.io/api/core/v1#ObjectReference

operator: Create new namespace instead of using default in console ku…

62be321

…ttl tests. Default namespace doesn't get cleaned up on succeeding tests But no way to reference the auto-generated kuttl namespace in Console clusterKeyRef spec

pvsune force-pushed the enterprise/pvsune/rp-console branch from 3860214 to 62be321 Compare September 8, 2022 06:25

RafalKorepta approved these changes Sep 8, 2022

View reviewed changes

pvsune merged commit 385261a into redpanda-data:dev Sep 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Console Google SSO support in operator #6282

Console Google SSO support in operator #6282

pvsune commented Aug 31, 2022 •

edited by andrewhsu

Loading

RafalKorepta left a comment

RafalKorepta Sep 2, 2022

RafalKorepta Sep 2, 2022

pvsune Sep 5, 2022

RafalKorepta Sep 2, 2022

pvsune Sep 5, 2022

RafalKorepta Sep 5, 2022

pvsune Sep 5, 2022

RafalKorepta Sep 2, 2022

pvsune Sep 5, 2022

RafalKorepta Sep 2, 2022

pvsune Sep 5, 2022

RafalKorepta Sep 5, 2022

pvsune Sep 5, 2022

RafalKorepta left a comment

		Name string `json:"name"`
		Namespace string `json:"namespace"`

Console Google SSO support in operator #6282

Console Google SSO support in operator #6282

Conversation

pvsune commented Aug 31, 2022 • edited by andrewhsu Loading

Cover letter

Backport Required

UX changes

Release notes

Features

Improvements

RafalKorepta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafalKorepta left a comment

Choose a reason for hiding this comment

pvsune commented Aug 31, 2022 •

edited by andrewhsu

Loading