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

Support for Azure Workload Identities #2363

Merged
merged 4 commits into from Dec 5, 2023
Merged

Conversation

Sean-Holcomb
Copy link
Contributor

@Sean-Holcomb Sean-Holcomb commented Dec 4, 2023

What does this PR change?

  • Add support for using DefaultAzureCredentials for azure blob authentication. This would remove the need for the creation of secrets in kubernetes.
  • Start using the blob provider from the Azure SDK for Go.
  • Update the Azure Authorizer to be service agnostic like the Authorizers of the other providers.

Does this PR relate to any other PRs?

replaces: #2117

How will this PR impact users?

  • Users will be able to configure Workload Identities to access CloudCosts

Does this PR address any GitHub or Zendesk issues?

  • Closes

How was this PR tested?

Image tested with Shared Key and Workload identity configurations for Azure Storage

Does this PR require changes to documentation?

  • Yes it will

Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?

Davidsoff and others added 2 commits December 4, 2023 15:05
Signed-off-by: David Soff <david@soff.nl>
Signed-off-by: Sean Holcomb <seanholcomb@gmail.com>
Copy link

vercel bot commented Dec 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
opencost ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2023 11:32pm

Copy link
Contributor

@nik-kc nik-kc left a comment

Choose a reason for hiding this comment

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

Very nice! Love the update to Azure SDK for Go. Could this be extended to entirely replace azure-storage-blob-go throughout OC?

And someone better acquainted with CloudCosts can chime in regarding the CC side of this PR, but overall this looks great.

Copy link
Contributor

@nikovacevic nikovacevic left a comment

Choose a reason for hiding this comment

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

Couple of small questions, but otherwise looks good

pkg/cloud/azure/storagebillingparser.go Show resolved Hide resolved
pkg/cloud/azure/storageconnection.go Outdated Show resolved Hide resolved
Signed-off-by: Sean Holcomb <seanholcomb@gmail.com>
@Sean-Holcomb
Copy link
Contributor Author

@nik-kc that would be great, but will require a little more effort with removing some of the thanos code in package storage

Copy link

sonarcloud bot commented Dec 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
7.6% 7.6% Duplication

@Sean-Holcomb Sean-Holcomb merged commit 8029e85 into develop Dec 5, 2023
7 checks passed
@chrisjohnson00
Copy link

Is there any place that this PR is added to the docs?
I don't see any mention here https://www.opencost.io/docs/configuration/azure about how to use a workload identity with OpenCost.
My normal way of doing it isn't working.

IE:

  • create an identity, assign the OpenCost role
  • Create OpenCost service account, associate it with Federated Identity Credentials for my AKS cluster
  • add the azure.workload.identity/use=true label to the deployment/pod

This results in the expected env vars being injected as well as the azure-identity-token mounted.

AZURE_CLIENT_ID:                       XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
AZURE_TENANT_ID:                       XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
AZURE_FEDERATED_TOKEN_FILE:            /var/run/secrets/azure/tokens/azure-identity-token
AZURE_AUTHORITY_HOST:                  https://login.microsoftonline.com/

Results in:

opencost 2024-02-08T00:54:03.805410641Z INF Failed to download pricing data: azure.BearerAuthorizer#WithAuthorization: Failed to refresh the Token for request to https://management.azure.com/subscriptions//providers/Microsoft.Commerce/RateCard?%!f(MISSING)ilter=OfferDurableId+eq+%!M(MISSING)S-AZR-0003p%!+(MISSING)and+Currency+eq+%!U(MISSING)SD%!+(MISSING)and+Locale+eq+%!e(MISSING)n-US%!+(MISSING)and+RegionInfo+eq+%!U(MISSING)S%!&(MISSING)api-version=2015-06-01-preview: StatusCode=400 -- Original Error: adal: Refresh request failed. Status Code = '400'. Response body: {"error":"invalid_request","error_description":"Identity not found"} Endpoint http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&client_id=XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX&resource=https%!A(MISSING)%!F(MISSING)%!F(MISSING)management.azure.com%!F(MISSING) 

@Sean-Holcomb
Copy link
Contributor Author

@chrisjohnson00 are you trying to set up CloudCost or RateCard?

@chrisjohnson00
Copy link

chrisjohnson00 commented Feb 21, 2024

@chrisjohnson00 are you trying to set up CloudCost or RateCard?

@Sean-Holcomb

Honestly, I don't know the difference between the two... there isn't much detail in the docs about what each is and when to use one vs the other.

I think RateCard, essentially I want to replace the default cost data with data from Azure. I'm only interested in getting costs of the Kubernetes cluster itself, so I don't think I need CloudCost?

@Sean-Holcomb
Copy link
Contributor Author

Yes it does sound like you are looking for rate card. Unfortunately this PR is specifically for Cloud Cost integration. The RateCard Api has an entirely different configuration pathway and I am not sure that there is a way to run it when running OpenCost from the helm chart. There is also currently no plan to move the older "Pricing Source" integrations to the newer configuration method.

@Sean-Holcomb Sean-Holcomb deleted the davidsoff/azure-configs branch February 22, 2024 18:33
@chrisjohnson00
Copy link

Thanks!

@cmergenthaler
Copy link

@Sean-Holcomb So when using CloudCost we can basically ignore the Failed to download pricing data RateCard API error?

@AjayTripathy
Copy link
Contributor

@Sean-Holcomb So when using CloudCost we can basically ignore the Failed to download pricing data RateCard API error?

That's fine yes.

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

Successfully merging this pull request may close these issues.

None yet

7 participants