Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

126 airflow cognito integration #226

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft

126 airflow cognito integration #226

wants to merge 10 commits into from

Conversation

nikki-t
Copy link
Collaborator

@nikki-t nikki-t commented Oct 11, 2024

Purpose

  • Integrate Cognito with Airflow (Flask AppBuilder) authentication so that users will only need to keep track of one set of credentials when interacting with the system.

Proposed Changes

  • [ADD] Cognito Airflow integration via the customization of webserver_config.py.

Issues

Testing

Implemented locally and then deployed to unity-venue-dev for testing:

  1. Confirmed hand off to Cognito for user authentication
    Screenshot 2024-10-11 at 08 49 28

  2. Confirmed log into Airflow UI and user role
    Screenshot 2024-10-11 at 08 50 35

@nikki-t
Copy link
Collaborator Author

nikki-t commented Oct 11, 2024

Pending items:

  1. I have enabled SSL on the load balancers as the Cognito app client requires an HTTPS address for the callback/redirect URIs. I am not sure if we want to enable this going forward or if there is a different URL we should use?
  2. I did not write any unit tests for webserver_config.py, is this something we would like to see in place?

@LucaCinquini
Copy link
Collaborator

I tested this branch on a custom "unity-luca-1" deployment and it worked - great job!
But we do need to address the SSL issue. For one thing, the OGC API will not work now because the custom deployment does not have a DNS name that matches the SSL certificate.

@jpl-btlunsfo
Copy link
Collaborator

Just a note regarding the venue-services proxy; I believe you'll need to modify the unity_proxy_airflow_ui and unity_proxy_ogc_api SSM parameters.

They've got http references that need to be https, and we'll need to add a SSLProxyEngine On directive as well, like so:

resource "aws_ssm_parameter" "unity_proxy_ogc_api" {
  name        = format("/%s", join("/", compact(["unity", var.project, var.venue, "cs", "management", "proxy", "configurations", "016-sps-ogc-api"])))
  description = "The unity-proxy configuration for the Airflow OGC API."
  type        = "String"
  value       = <<-EOT

    SSLProxyEngine On
    <Location "/${var.project}/${var.venue}/ogc/">
      ProxyPassReverse "/"
    </Location>
    <LocationMatch "^/${var.project}/${var.venue}/ogc/(.*)$">
      ProxyPassMatch "https://${data.kubernetes_ingress_v1.ogc_processes_api_ingress_internal.status[0].load_balancer[0].ingress[0].hostname}:5001/$1"
      ProxyPreserveHost On

@nikki-t
Copy link
Collaborator Author

nikki-t commented Oct 14, 2024

Modify Terraform to query SSM parameter store for parameter that contain Cognito app client data. Can confirm that sensitive data is not shown when running Terraform plan.

Terraform will perform the following actions:

  # module.unity-sps-airflow.helm_release.airflow will be updated in-place
  ~ resource "helm_release" "airflow" {
        id                         = "airflow"
        name                       = "airflow"
      ~ values                     = [
          # Warning: this attribute value will be marked as sensitive and will not
          # display in UI output after applying this change. The value is unchanged.
          ~ (sensitive value),
        ]
        # (27 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

Waiting on how the client id and client secret will be stored in SSM parameter store and whether it falls under SPS to store these values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants