Azure / application-gateway-kubernetes-ingress

This is an ingress controller that can be run on Azure Kubernetes Service (AKS) to allow an Azure Application Gateway to act as the ingress for an AKS cluster.
https://azure.github.io/application-gateway-kubernetes-ingress
MIT License
677 stars 420 forks source link

feat(helm-chart/servicePrincipal): allow an existingSecret for holding armAuth.json #1551

Closed eyenx closed 10 months ago

eyenx commented 1 year ago

Checklist

Description

As of today, when using the servicePrincipal method for achieving authentication, the secretJSON needs to be added as value in the chart.

With this change, the secret can already exist, which allows us to grab it from an external source like Azure Key Vault....

This way, the secretJSON value which is sensitive does not need to be added to the values and can be grabbed directly from an already existing Kubernetes Seecret.

Fixes

Unable to deploy AGIC without exposing the secretJSON in the Helm values.

Signed-off-by: Toni Tauro toni.tauro@adfinis.com

eyenx commented 1 year ago

Any Updates on this? @akshaysngupta

akshaysngupta commented 1 year ago

@eyenx can you share if you have tested this change ?

eyenx commented 1 year ago

@akshaysngupta I added unittests for this explicit example.

=== RUN   TestChart
=== RUN   TestChart/sample-config-empty
=== RUN   TestChart/sample-config-existing-secret
=== RUN   TestChart/sample-config-prohibited-target
=== RUN   TestChart/sample-config-workload-identity
=== RUN   TestChart/sample-config
--- PASS: TestChart (0.24s)
    --- PASS: TestChart/sample-config-empty (0.05s)
    --- PASS: TestChart/sample-config-existing-secret (0.04s)
    --- PASS: TestChart/sample-config-prohibited-target (0.05s)
    --- PASS: TestChart/sample-config-workload-identity (0.05s)
    --- PASS: TestChart/sample-config (0.05s)
PASS
ok      github.com/Azure/application-gateway-kubernetes-ingress/helm/ingress-azure/tests        (cached)
eyenx commented 1 year ago

Any Update @akshaysngupta

eyenx commented 1 year ago

Any Update @akshaysngupta ?

eyenx commented 1 year ago

Any Update @akshaysngupta ?

eyenx commented 1 year ago

Any Update @akshaysngupta ?

ant-wtrog commented 11 months ago

Hi @akshaysngupta, is there a chance to merge these changes in the near future? We're stuck at this point and would want to avoid coding a workaround.

Best Regards Wolfgang

eyenx commented 10 months ago

Seems like Microsoft does not care about this. @akshaysngupta

Not even a reaction if it is in scope or not.

Really sad.

akshaysngupta commented 10 months ago

@eyenx sorry for delaying this PR so much. It looks perfectly fine. I have added a minor comment.