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

Resurrect Workload Identity Work #6802

Merged
merged 24 commits into from
Jul 1, 2024

Conversation

ledbutter
Copy link
Contributor

Why the changes in this PR are needed?

To allow for Azure kubernetes deployments to utilize workload identity, their newer approach to federating with identity providers.

What are the changes in this PR?

Adding new client credential attributes to support workload identity.

Notes to assist PR review:

Based on discussion in #6012

Further comments:

This is resurrecting the work started in #6014 and closed without explanation. Given the specialized support for AWS KMS, I could foresee the original approach here should be changed to more closely follow that approach.

Copy link

netlify bot commented Jun 10, 2024

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 23a9f9a
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/6682c2be479cc10008692d86
😎 Deploy Preview https://deploy-preview-6802--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ledbutter and others added 10 commits June 10, 2024 10:29
…s_gc_cpu_fraction`

Prometheus appears to have dropped the `go_memstats_gc_cpu_fraction`
metric, since at least Go 1.18. This commit drops the missing metric
from the table of expected metrics for OPA on the monitoring docs page.

Reference: prometheus/client_golang#1500

Fixes: open-policy-agent#6783

Signed-off-by: Philip Conrad <[email protected]>
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.25.0 to 0.26.0.
- [Commits](golang/net@v0.25.0...v0.26.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github.com/containerd/containerd](https://github.com/containerd/containerd) from 1.7.17 to 1.7.18.
- [Release notes](https://github.com/containerd/containerd/releases)
- [Changelog](https://github.com/containerd/containerd/blob/main/RELEASES.md)
- [Commits](containerd/containerd@v1.7.17...v1.7.18)

---
updated-dependencies:
- dependency-name: github.com/containerd/containerd
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [aquasecurity/trivy-action](https://github.com/aquasecurity/trivy-action) from 0.21.0 to 0.22.0.
- [Release notes](https://github.com/aquasecurity/trivy-action/releases)
- [Commits](aquasecurity/trivy-action@0.21.0...0.22.0)

---
updated-dependencies:
- dependency-name: aquasecurity/trivy-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Sven Grosen <[email protected]>
anderseknert
anderseknert previously approved these changes Jun 12, 2024
Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! The change looks good to me. I'll assume you've tested this against Azure :)

@ledbutter
Copy link
Contributor Author

Thanks for working on this! The change looks good to me. I'll assume you've tested this against Azure :)

Actually an unsafe assumption: we are still in the evaluation phase for OPA so no established deployments but we do have Azure environments with workload identity configured so I will try this out tomorrow.

Signed-off-by: Sven Grosen <[email protected]>
Signed-off-by: Sven Grosen <[email protected]>
@ledbutter
Copy link
Contributor Author

Thanks for working on this! The change looks good to me. I'll assume you've tested this against Azure :)

Actually an unsafe assumption: we are still in the evaluation phase for OPA so no established deployments but we do have Azure environments with workload identity configured so I will try this out tomorrow.

I'm struggling to get the docker image to build locally (Mac M1, corporate network blocking download of the chainguard base images), so I've been trying to grab the image generated via the workflow and deploy it to my AKS cluster, but I'm seeing unexpected behavior (leading me to believe it isn't actually my work):

services:
  azure_storage_account:
    url: ${STORAGE_ACCOUNT_URL}
    headers:
      x-ms-version: 2017-11-09
    response_header_timeout_seconds: 5
    credentials:
      oauth2:
        grant_type: client_credentials
        client_id: ${AZURE_CLIENT_ID}
        client_assertion_path: /var/run/secrets/azure/tokens/azure-identity-token
        token_url: https://login.microsoftonline.com/${TENANT_ID}/oauth2/v2.0/token
        scopes:
          - https://storage.azure.com/.default

bundles:
  authz:
    service: azure_storage_account
    resource: opa/policy.rego
    persist: true
    polling:
      min_delay_seconds: 60
      max_delay_seconds: 120

image

Is there a reliable image tag generated from my PR branch that I can reference?

I've pushed the latest tests to try to validate the rest-level config is getting picked up properly.

@ledbutter
Copy link
Contributor Author

Further confirming this, if I just run the binary locally, I get an expected error (since I don't have that file locally):
image

@ledbutter
Copy link
Contributor Author

@anderseknert Ok, after much finagling, I got this working (with the latest changes to include client_id in the body):

image

Service config:

services:
  azure_storage_account:
    url: https://${MY_STORAGE_ACCOUNT}.blob.core.windows.net/
    headers:
      x-ms-version: 2017-11-09
    response_header_timeout_seconds: 5
    credentials:
      oauth2:
        grant_type: client_credentials
        client_id: "${AZURE_CLIENT_ID}"
        client_assertion_path: "${AZURE_FEDERATED_TOKEN_FILE}"
        token_url: "${AZURE_AUTHORITY_HOST}/${AZURE_TENANT_ID}/oauth2/v2.0/token"
        scopes:
          - https://storage.azure.com/.default

bundles:
  authz:
    service: azure_storage_account
    resource: opa/policies.tar.gz
    persist: true
    polling:
      min_delay_seconds: 60
      max_delay_seconds: 120

deployment:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: opa
  labels:
    app: opa
spec:
  replicas: 1
  selector:
    matchLabels:
      app: opa
  template:
    metadata:
      labels:
        app: opa
        azure.workload.identity/use: "true"
      name: opa
    spec:
      serviceAccountName: ${my_workload_identity_service}
      containers:
      - name: opa
        image: ${OPA_IMAGE}
        imagePullPolicy: Always
        ports:
        - name: http
          containerPort: 8181
        args:
        - "run"
        - "--ignore=.*"  # exclude hidden dirs created by Kubernetes
        - "--server"
        - "--config-file=/config/service.yml"
        volumeMounts:
        - readOnly: true
          mountPath: /config
          name: config
        - mountPath: /.opa
          name: opa-volume
      volumes:
      - name: config
        configMap:
          name: workload-config
      - name: opa-volume
        emptyDir: {}

@anderseknert
Copy link
Member

@ledbutter sorry for my delayed response! Happy to hear that! I'll take a closer look tonight 👍

Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me! There's no documentation provided in this PR though. Could you please add docs for the new configuration attributes, and perhaps something like the example you provided here for how to set this up? Thanks!

@ledbutter
Copy link
Contributor Author

Code looks good to me! There's no documentation provided in this PR though. Could you please add docs for the new configuration attributes, and perhaps something like the example you provided here for how to set this up? Thanks!

I added some docs, I tried to follow the existing layout/format, let me know if you want something more involved.

Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you @ledbutter 👏

@anderseknert anderseknert merged commit e2721d3 into open-policy-agent:main Jul 1, 2024
28 checks passed
Manish-Giri pushed a commit to Manish-Giri/opa that referenced this pull request Jul 2, 2024
Add support for using Azure Workload Identity authentication.

Signed-off-by: Sven Grosen <[email protected]>
Signed-off-by: Manish Giri <[email protected]>
@ledbutter ledbutter deleted the azureWorkloadIdentity branch July 2, 2024 16:17
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.

Support Azure workload identity to access Azure resources (storage blobs, etc)
6 participants