acryldata / datahub-helm

Repository of helm charts for deploying DataHub on a Kubernetes cluster
Apache License 2.0
159 stars 239 forks source link

GMS authentication breaks datahub-system-update-job #265

Closed matthijsvanderloos closed 1 year ago

matthijsvanderloos commented 1 year ago

Describe the bug

When enabling GMS authentication (via global.datahub.metadata_service_authentication.enabled), the datahub-system-update-job job fails to start on a fresh install of Datahub because it references the datahub-auth-secrets secret, which has not yet been created.

To Reproduce Run the following commands:

helm repo add datahub https://helm.datahubproject.io/
helm template datahub datahub/datahub --set global.datahub.metadata_service_authentication.enabled=true

The output includes the spec for the datahub-system-update-job job. Note the reference to datahub-auth-secrets for the DATAHUB_SYSTEM_CLIENT_SECRET environment variable:

---
# Source: datahub/templates/datahub-upgrade/datahub-system-update-job.yml
apiVersion: batch/v1
kind: Job
metadata:
  name: datahub-datahub-system-update-job
  labels:
    app.kubernetes.io/managed-by: "Helm"
    app.kubernetes.io/instance: "datahub"
    app.kubernetes.io/version: 0.10.0
    helm.sh/chart: "datahub-0.2.150"
  annotations:
    # This is what defines this resource as a hook. Without this line, the
    # job is considered part of the release.
    "helm.sh/hook": pre-install,pre-upgrade
    "helm.sh/hook-weight": "-4"
    "helm.sh/hook-delete-policy": before-hook-creation
spec:
  template:
    spec:
      volumes:
      restartPolicy: Never
      securityContext:
        {}
      initContainers:
      containers:
        - name: datahub-system-update-job
          image: "acryldata/datahub-upgrade:v0.10.0"
          imagePullPolicy: IfNotPresent
          args:
            - "-u"
            - "SystemUpdate"
          env:
            - name: DATAHUB_UPGRADE_HISTORY_TOPIC_NAME
              value: DataHubUpgradeHistory_v1
            - name: DATAHUB_REVISION
              value: "1"
            - name: ENTITY_REGISTRY_CONFIG_PATH
              value: /datahub/datahub-gms/resources/entity-registry.yml
            - name: DATAHUB_GMS_HOST
              value: datahub-datahub-gms
            - name: DATAHUB_GMS_PORT
              value: "8080"
            - name: DATAHUB_MAE_CONSUMER_HOST
              value: datahub-datahub-mae-consumer
            - name: DATAHUB_MAE_CONSUMER_PORT
              value: "9091"
            - name: EBEAN_DATASOURCE_USERNAME
              value: "root"
            - name: EBEAN_DATASOURCE_PASSWORD
              valueFrom:
                secretKeyRef:
                  name: "mysql-secrets"
                  key: "mysql-root-password"
            - name: EBEAN_DATASOURCE_HOST
              value: "prerequisites-mysql:3306"
            - name: EBEAN_DATASOURCE_URL
              value: "jdbc:mysql://prerequisites-mysql:3306/datahub?verifyServerCertificate=false&useSSL=true&useUnicode=yes&characterEncoding=UTF-8&enabledTLSProtocols=TLSv1.2"
            - name: EBEAN_DATASOURCE_DRIVER
              value: "com.mysql.cj.jdbc.Driver"
            - name: DATAHUB_SYSTEM_CLIENT_ID
              value: __datahub_system
            - name: DATAHUB_SYSTEM_CLIENT_SECRET
              valueFrom:
                secretKeyRef:
                  name: datahub-auth-secrets
                  key: token_service_signing_key
            - name: KAFKA_BOOTSTRAP_SERVER
              value: "prerequisites-kafka:9092"
            - name: KAFKA_SCHEMAREGISTRY_URL
              value: "http://prerequisites-cp-schema-registry:8081"
            - name: ELASTICSEARCH_HOST
              value: "elasticsearch-master"
            - name: ELASTICSEARCH_PORT
              value: "9200"
            - name: SKIP_ELASTICSEARCH_CHECK
              value: "false"
            - name: ELASTICSEARCH_INSECURE
              value: "false"
            - name: ELASTICSEARCH_USE_SSL
              value: "false"
            - name: GRAPH_SERVICE_IMPL
              value: elasticsearch
            - name: METADATA_CHANGE_EVENT_NAME
              value: MetadataChangeEvent_v4
            - name: FAILED_METADATA_CHANGE_EVENT_NAME
              value: FailedMetadataChangeEvent_v4
            - name: METADATA_AUDIT_EVENT_NAME
              value: MetadataAuditEvent_v4
            - name: METADATA_CHANGE_PROPOSAL_TOPIC_NAME
              value: MetadataChangeProposal_v1
            - name: FAILED_METADATA_CHANGE_PROPOSAL_TOPIC_NAME
              value: FailedMetadataChangeProposal_v1
            - name: METADATA_CHANGE_LOG_VERSIONED_TOPIC_NAME
              value: MetadataChangeLog_Versioned_v1
            - name: METADATA_CHANGE_LOG_TIMESERIES_TOPIC_NAME
              value: MetadataChangeLog_Timeseries_v1
            - name: DATAHUB_UPGRADE_HISTORY_TOPIC_NAME
              value: DataHubUpgradeHistory_v1
            - name: DATAHUB_ANALYTICS_ENABLED
              value: "true"
            - name: ENTITY_REGISTRY_CONFIG_PATH
              value: /datahub/datahub-gms/resources/entity-registry.yml
            - name: EBEAN_DATASOURCE_USERNAME
              value: "root"
            - name: EBEAN_DATASOURCE_PASSWORD
              valueFrom:
                secretKeyRef:
                  name: "mysql-secrets"
                  key: "mysql-root-password"
            - name: EBEAN_DATASOURCE_HOST
              value: "prerequisites-mysql:3306"
            - name: EBEAN_DATASOURCE_URL
              value: "jdbc:mysql://prerequisites-mysql:3306/datahub?verifyServerCertificate=false&useSSL=true&useUnicode=yes&characterEncoding=UTF-8&enabledTLSProtocols=TLSv1.2"
            - name: EBEAN_DATASOURCE_DRIVER
              value: "com.mysql.cj.jdbc.Driver"
            - name: KAFKA_BOOTSTRAP_SERVER
              value: "prerequisites-kafka:9092"
            - name: KAFKA_SCHEMAREGISTRY_URL
              value: "http://prerequisites-cp-schema-registry:8081"
            - name: SCHEMA_REGISTRY_TYPE
              value: "KAFKA"
            - name: ELASTICSEARCH_HOST
              value: "elasticsearch-master"
            - name: ELASTICSEARCH_PORT
              value: "9200"
            - name: SKIP_ELASTICSEARCH_CHECK
              value: "false"
            - name: ELASTICSEARCH_USE_SSL
              value: "false"
            - name: ELASTICSEARCH_BUILD_INDICES_CLONE_INDICES
              value: "true"
            - name: ELASTICSEARCH_INDEX_BUILDER_MAPPINGS_REINDEX
              value: "true"
            - name: ELASTICSEARCH_INDEX_BUILDER_SETTINGS_REINDEX
              value: "true"
            - name: GRAPH_SERVICE_IMPL
              value: elasticsearch
          securityContext:
            {}
          volumeMounts:
          resources:
            limits:
              cpu: 500m
              memory: 512Mi
            requests:
              cpu: 300m
              memory: 256Mi

Expected behavior datahub-system-update-job job starts and completes successfully during fresh install with GMS authentication enabled.

Additional context In addition to this issue, there are also duplicated environment variables in the job spec, see #261.

HannesHil commented 1 year ago

This issue was introduced trough the usage as a pre-install hook in https://github.com/acryldata/datahub-helm/blob/0361995de9581d822fcd2b93f5a710d9a1e5a401/charts/datahub/templates/datahub-upgrade/datahub-system-update-job.yml#L14 Is is even necessary to have it here as a pre-install hook? It is only used to upgrade existing environments. Besides that one could think about creating the secret in a pre-install hook too.

HannesHil commented 1 year ago

Since creating the datahub-auth-secrets in a hook is not an option due to the "helm.sh/hook-delete-policy".

  1. One could either try to generate the secret separately like the neo4j or the mysql secrets.
  2. Or one could remove the pre-install from the upgrade job.
nachiket-juneja commented 1 year ago

Any updates on this?

mingz-work commented 1 year ago

Is there a temp fix or work around for this?

matthijsvanderloos commented 1 year ago

Is there a temp fix or work around for this?

As also mentioned by @HannesHil, the only viable option if you don't want to disable the update job is to (manually) generate the secrets up front and disable provisioning the secrets through the chart:

https://github.com/acryldata/datahub-helm/blob/f2269e3649c4e131f62a4c0e0bedbe4764628a54/charts/datahub/values.yaml#L393-L395

justmike1 commented 1 year ago

I created 2 PRs which adds ability to remove helmhooks with everything still running normally (with the upgrade jobs) but I got no feedback and I kept adding to the PR because I am using my fork. If you'd like I can update my fork again to what I use and I will provide the registry to pull from. @mingz-work @nachiket-juneja @matthijsvanderloos

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

github-actions[bot] commented 1 year ago

This issue was closed because it has been inactive for 30 days since being marked as stale.