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

enable key vault provider and workload identities on aks cluster #17

Merged

Conversation

Lusengeri
Copy link
Contributor

Enable Azure Key Vault Provider and Workload Identities on AKS Cluster

Description

This PR introduces the following changes to the AKS cluster:

  1. Azure Key Vault Provider: Integrates the Azure Key Vault provider to allow AKS workloads to access secrets stored in Azure Key Vault securely.

  2. Workload Identities: Enables Azure AD workload identities, allowing AKS pods to authenticate with Azure AD and securely access Azure resources (e.g., Key Vault) without managing service principal secrets.

Motivation

  • Operational Efficiency: This integration simplifies the secret management process. AKS workloads will be able to access specific secret vaults. K8s manifests can then be used in helm charts that directly access secrets. Dynamic reloading of pods will be possible when secrets are edited on the Azure Key Vault, negating the need to re-run deploy pipelines.

Changes Made

  • Key Vault Provider Configuration:

    • Added azure-keyvault-secrets provider in AKS configuration.
  • Workload Identities:

    • Enabled workload identity usage in the AKS Terraform module.

Related Issues

Issue #16

@batpad
Copy link
Contributor

batpad commented Oct 10, 2024

@Lusengeri this looks good to me, thank you!

@sunu can you please give a review and 👍 before we merge?

@szabozoltan69 once we merge into develop, let's check staging.

Slightly tangential question:

In these lines https://github.com/IFRCGo/go-deploy/pull/17/files#diff-66b3825c526e651280dbec69b73c391fb3927b607957b4cbbadab0715b0e4dedR2 we set terraform to ignore lifecycle changes on the AKS cluster. I've had some bad experiences in the past where terraform has done unexpected things due to subtle upstream version changes, etc. so rather like to make changes to the cluster explicit.

@Lusengeri any thoughts on whether, after this deploy, we should comment those lines back in and ignore changes there, or if it's okay? We can discuss that separately - right now I think this is fine to merge, but let's wait for @sunu to have his eyes on it.

Thanks @Lusengeri !

@batpad batpad requested a review from sunu October 10, 2024 07:00
@szabozoltan69
Copy link
Collaborator

Should we fire up an Azure KeyVault for this change, and upload TLS cert(s) manually? (Or is it due only after the deployment?)

@Lusengeri
Copy link
Contributor Author

Lusengeri commented Oct 11, 2024

@batpad , I understand your concern. I feared as much and I did a local terraform plan; only the intended changes appeared at that point. Since our cluster version and terraform provider versions are pinned down, I think we've minimized the chance of 'stealth' updates. I think we're ok with that as far as this change is concerned. We can return the 'ignore-changes' block after applying this.

@szabozoltan69 , the intention is to have a vault per application. Prior to deploying any application we would need to create a vault for it and add the necessary secrets. I will be pushing a terraform module that will ease this process. The currently running application doesn't have to change at this point. However, in the future we can adjust it such that all necessary secrets are stored in Azure Vault.

@szabozoltan69
Copy link
Collaborator

For staging I created a vault in its resource group – if not required, I can delete it.

Copy link
Collaborator

@sunu sunu left a comment

Choose a reason for hiding this comment

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

Looks good to me! I would second the suggestion that we add back the 'ignore-changes' block after this PR is merged and deployed.

@batpad
Copy link
Contributor

batpad commented Oct 16, 2024

@Lusengeri @szabozoltan69 - what are next steps here? Do we need to plan a call and do this merge when we're together, or can we just merge and deal with things?

@Lusengeri slightly tangential question: we have a KeyVault per application, but is it also possible to have a "global KeyVault" of sorts with "Shared Secrets"? There are likely to be some secrets that need to be shared across different applications, in which case it's nice to be able to have them in one place, rather than needing to update the same secret in multiple KeyVaults if that secret was to change. Currently, an example is the *.ifrc.org wildcard SSL certificate, but I imagine there might be others. Ofc, this is not a blocker, am mostly just curious if there's a good solution for these.

@szabozoltan69
Copy link
Collaborator

szabozoltan69 commented Oct 16, 2024

@batpad it is still not clear when/where should I put the recent variables (and how to gain those which are secret ones and not in the bash ENV). And how to point to the (in Staging) created Azure key vault (or was that manual creation step unnecessary).

@batpad
Copy link
Contributor

batpad commented Oct 16, 2024

@Lusengeri @szabozoltan69 - should we get on a call to clarify these things and make sure things are clear?

Although, I believe with this PR merge, nothing should change for the GO API application - this would just enable the KeyVault for the Risk Module application. Setting up the GO API to use the KeyVault would be a separate step (I believe), so maybe we can worry about these when we are doing that step, and go ahead with this to move ahead with the Risk Module deploy and testing?

@szabozoltan69
Copy link
Collaborator

Ok, then I merge and deploy this one now without any variable setting/copying. Thanks @batpad !

@szabozoltan69 szabozoltan69 merged commit 2cb2865 into develop Oct 16, 2024
@szabozoltan69 szabozoltan69 deleted the feature/aks-workload-id-and-key-vault-changes branch October 16, 2024 11:54
@szabozoltan69
Copy link
Collaborator

Deployment to Staging is OK.

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.

4 participants