flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.38k stars 573 forks source link

[BUG] Flyte-binary Helm misconfigures flyte-binary pod config when auth.externalAuthServer: true, and auth.internal.clientId is undefined in values.yaml #3659

Open PudgyPigeon opened 1 year ago

PudgyPigeon commented 1 year ago

Related issues

3660 #3661

Describe the bug

When enabling external authorization servers for the Flyte-binary deployment, if auth.externalAuthServer = true in the values.yaml file, and if auth.internal.clientId is undefined (defaults to flytepropeller), the flyte-binary pod will query the external authorization server with the flytepropeller parameter as the clientId.

This results in failed auth to the server when scheduling workflows/tasks/notebook tasks through pyflyte or the web console. Workflows and tasks will show up as UNKNOWN in the web console, and the flyte-binary pod will continuously make requests to the auth server - too many retries(? Have counted potentially 60 within a span of a minute to auth0,).

Specifically, within the flyte-binary pod: if you exec into the flyte container in the pod at path: /etc/flyte/config.d/000-core.yaml or a similar path, the relevant parameters within the admin field aren't set correctly.

Or alternatively, add more documentation to clearly outline this behaviour so future users are aware of how these parameters are set - specifically within the context of integrating Auth0.

Tldr;

Granted, this could be classified as a documentation issue, but perhaps the internal key could be renamed to something along the lines of propeller or dataplane (not sure if this is the correct terminology) in order to clarify what is being configured with internal.

Expected behavior

When auth.enableAuthServer: false, it might be a good idea to make internal.clientId be a required parameter. In addition to that, it might help to indicate to users that this will then correspond to 000-core.yaml and will be used by the scheduler for workflow pods.

The current documentation implies that this only needs to be set for internal use and makes no mention of it being linked to external.

One alternative is if auth.enableAuthserver: false, the Helm chart template could derive the values for 000-core.yaml -- admin.clientId from a different field in the values.yaml - thirdpartyconfig.flyteclient is one suggestion.

Relevant path in the helm chart is templates/configmap.yaml

Additional context to reproduce

## values.yaml
  auth: 
    enabled: true
    enableAuthServer: false
    internal:
#### Note that clientId is not set - will default to flytepropeller which is unwanted behaviour
      clientSecret: <client secret here>  #
      clientSecretHash: <client hash>
    oidc:
      baseUrl: <external auth server>
      clientId: <clientId>
      clientSecret: <client secret>
    flyteClient:
      clientId: <clientId>
      redirectUri: http://localhost:53593/callback
      scopes:
        - read:client_grants
      audience: <audience of your api>
    authorizedUris:
      - put the necessary uris here
## in flyte-binary pod in K8S - /etc/flyte/config.d/000-core.yaml
admin:
  clientId: flytepropeller ### This would ideally be the clientId for auth server. Can be set with internal.clientId but unclear as to how to affect this specific parameter with current docs
  endpoint: localhost:8089
  insecure: true
catalog-cache:  
  endpoint: localhost:8081
  insecure: true
  type: datacatalog
cluster_resources:
  standaloneDeployment: false
  templatePath: /etc/flyte/cluster-resource-templates
logger:
  show-source: true
  level: 1
propeller:
  create-flyteworkflow-crd: true
webhook:
  certDir: /var/run/flyte/certs
  localCert: true
  secretName: flyte-backend-flyte-binary-webhook-secret
  serviceName: flyte-backend-flyte-binary-webhook
  servicePort: 443
## /etc/flyte/config.d/004-auth.yaml

data: 
  auth:
    userAuth:
      openId:
        clientId: AUTH0_CLIENT_ID
        baseUrl: AUTH0_BASE_URL
        scopes:
          - profile
          - openid
          - offline_access
    appAuth:  
      authServerType: External
      externalAuthServer:
        baseUrl: AUTH0_BASE_URL
        metadataUrl: .well-known/openid-configuration
        allowedAudience: AUTH0_AUDIENCE
      thirdPartyConfig:
        flyteClient:
          clientId: AUTH0_CLIENT_ID
          redirectUri: http://localhost:53593/callback
          audience: AUTH0_AUDIENCE
          scopes:
            - read:client_grants
    authorizedUris:
      - http://flyteadmin:80
      - http://flyteadmin.flyte.svc.cluster.local:80
      - http://flyteadmin.mlops-services.svc.cluster.local:80
      - http://HELM_NAME-flyte-binary.flyte.svc.cluster.local:80
      - http://HELM_NAME-flyte-binary.flyte.svc:80
      - http://HELM_NAME-flyte-binary.flyte:80
      - http://HELM_NAME-flyte-binary:80
      - http://localhost:8089
      -  other URIs can go here as well
  server: 
    security:
      secure: false
      useAuth: true

Screenshots

No response

Are you sure this issue hasn't been raised already?

Have you read the Code of Conduct?

welcome[bot] commented 1 year ago

Thank you for opening your first issue here! 🛠

eapolinario commented 8 months ago

@davidmirror-ops , is this still applicable?

davidmirror-ops commented 3 weeks ago

I think this is still relevant and still a docs issue for the most part