oauth2-proxy / manifests

For hosting manifests to allow for the deployment of OAuth2-Proxy/OAuth2-Proxy
Apache License 2.0
172 stars 155 forks source link

sessionStorage.redis.existingSecret does not correctly set OAUTH2_PROXY_REDIS_PASSWORD #266

Closed Schueni1 closed 2 days ago

Schueni1 commented 3 days ago

sessionStorage.redis.existingSecret is set to redis, but the result is release-name-redis

        - name: OAUTH2_PROXY_REDIS_PASSWORD
          valueFrom:
            secretKeyRef:
              name: release-name-redis
              key: redis-password
tuunit commented 3 days ago

Hi @Schueni1,

please add more context when creating a PR to reproduce an issue. It is not clear what you mean by "existingSecret is set to redis". The existingSecret in the values.yaml is not set to redis.

If you set the sessionStorage.redis.existingSecret to any value it is inserted as such:

https://github.com/oauth2-proxy/manifests/blob/49c2289c450d557b14653e4c3e84abe436be5221/helm/oauth2-proxy/templates/deployment.yaml#L197-L201

Schueni1 commented 3 days ago

Sorry. I meant, that I configured it to "redis". But that resulted in the name being set to "release-name-redis" and not just "redis".

tuunit commented 3 days ago

Sorry. I meant, that I configured it to "redis". But that resulted in the name being set to "release-name-redis" and not just "redis".

@Schueni1 I don't see how that is possible. We directly reference the value without any processing.

Which version of the helm chart are you using? Can you provide your values.yaml? (obviously obfuscate the security sensitive parts)

Schueni1 commented 3 days ago
affinity:
  podAntiAffinity:
    requiredDuringSchedulingIgnoredDuringExecution:
      - labelSelector:
          matchExpressions:
            - key: app
              operator: In
              values:
                - oauth2-proxy
        topologyKey: kubernetes.io/hostname
config:
  existingSecret: oauth2-proxy
extraArgs:
  allowed-group: abc ,local-oauthprx-htpasswd-group
  cookie-domain: .somename.domain.com
  cookie-secure: true
  email-domain: "*"
  htpasswd-user-group: local-oauthprx-htpasswd-group
  insecure-oidc-skip-nonce: false
  oidc-email-claim: email
  oidc-groups-claim: groups
  oidc-issuer-url: https://sub.domain.com/abc
  provider: oidc
  redirect-url: https://someurl/oauth2/callback
  reverse-proxy: true
  set-xauthrequest: true
  silence-ping-logging: true
  upstream: file:///dev/null
  user-id-claim: email
  whitelist-domain: .somename.domain.com
  # Additionally authenticate against a htpasswd file. Entries must be created with "htpasswd -s" for SHA encryption.
  # Alternatively supply an existing secret which contains the required information.
htpasswdFile:
  enabled: true
  existingSecret: oauth2-proxy-htpasswd-entries
ingress:
  enabled: true
  hosts:
    - oauth.somename.domain.com
initContainers:
  # if the redis sub-chart is enabled, wait for it to be ready
  # before starting the proxy
  # creates a role binding to get, list, watch, the redis master pod
  # if service account is enabled
  waitForRedis:
    enable: true
    resources:
      limits:
        cpu: 100m
        memory: 300Mi
      requests:
        cpu: 100m
metrics:
  servicemonitor:
    enabled: true
    labels:
      prometheus: kps
podDisruptionBudget:
    enabled: true
redis:
  architecture: replication
  sentinel:
    enabled: true
    redisShutdownWaitFailover: true
  replica:
    replicaCount: 3
  auth:
    existingSecret: redis
  enabled: true
  global:
    storageClass: ceph-c2-prod-rz01-rbd-somename-1
  master:
    persistence:
      size: 1Gi
    resources:
      limits:
        cpu: 100m
        memory: 100Mi
      requests:
        cpu: 10m
        memory: 10Mi
  metrics:
    enabled: true
    resources:
      limits:
        cpu: 100m
        memory: 100Mi
      requests:
        cpu: 10m
        memory: 10Mi
    serviceMonitor:
      enabled: false
      selector:
        prometheus: kps
replicaCount: 2
resources:
  limits:
    cpu: 100m
    memory: 100Mi
  requests:
    cpu: 10m
    memory: 30Mi
sessionStorage:
  type: redis
  existingSecret: "redis"
  passwordKey: "redis-password"
  redis:
    clientType: sentinel
    standalone:
      connectionUrl: "redis://oauth2-proxy-redis:6379"
    sentinel:
      existingSecret: "redis"
      passwordKey: "redis-password"
      masterName: mymaster
      connectionUrls: ["redis://oauth2-proxy-redis:6379"]
tuunit commented 3 days ago

I tested it with your values and get:

image

Schueni1 commented 2 days ago

I tested with 7.7.30 as well as with 7.8.0. Both versions have the same bug.

Your picture shows OAUTH2_PROXYREDISSENTINEL_PASSWORD and not OAUTH2_PROXY_REDIS_PASSWORD.

        - name: OAUTH2_PROXY_SESSION_STORE_TYPE
          value: "redis"
        - name: OAUTH2_PROXY_REDIS_PASSWORD
          valueFrom:
            secretKeyRef:
              name: release-name-redis
              key: redis-password
        - name: OAUTH2_PROXY_REDIS_USE_SENTINEL
          value: "true"
tuunit commented 2 days ago

Yes because in your provided config you only set the following:

sessionStorage:
  type: redis
  existingSecret: "redis"
  passwordKey: "redis-password"
  redis:
    clientType: sentinel
    standalone:
      connectionUrl: "redis://oauth2-proxy-redis:6379"
    sentinel:
      existingSecret: "redis"
      passwordKey: "redis-password"
      masterName: mymaster
      connectionUrls: ["redis://oauth2-proxy-redis:6379"]

You are setting sessionStorage.existingSecret which doesn't exist and sessionStorage.redis.sentinel.existingSecret. You are NOT setting sessionStorage.redis.existingSecret

Schueni1 commented 2 days ago

Oh! You are completely right! I configured it in the wrong place. I think this issue can be closed. Sorry 😅

Thanks for the patience and the help!

tuunit commented 2 days ago

Happens