argo-workflows icon indicating copy to clipboard operation
argo-workflows copied to clipboard

fix: Allow TLS secret configuration to be specified on the command line. Fixes #5582

Open lnattrass opened this issue 2 years ago • 12 comments

Fixes #5582

Additionally, secrets (particularly those from cert-manager) contain a ca.crt which is now added to the certificate trusts, meaning that the issue in #7632 should also be fixed properly, allowing privately signed certificates to be used by argo server.

I tested these changes by running argo server locally and confirming that when ca.crt is added to the TLSConfig as a trusted cert, the grpc-gateway endpoint /api/v1/info returns a valid response. The UI appears functional, etc.

lnattrass avatar Feb 14 '22 20:02 lnattrass

feat: adding support for getting tls certificates from kubernetes secret (e.g. (#7621)

alexec avatar Feb 16 '22 23:02 alexec

@ChaosInTheCRD could you please take a look at this?

alexec avatar Feb 16 '22 23:02 alexec

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 02 '22 17:03 stale[bot]

Can I propose we have a small PR with the fix for the flag, which we know we can merge, and then a second PR to discussed the complex bit?

Are we going to create a separate PR as @alexec suggested? Otherwise, I suggest in the worst case that the PR should be merged.

ChaosInTheCRD avatar Mar 02 '22 17:03 ChaosInTheCRD

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 12 '22 12:03 stale[bot]

@ChaosInTheCRD did you want to open a separate PR to fix the parameter?

alexec avatar Apr 14 '22 14:04 alexec

@lnattrass @alexec Is it available with new argo-workflow 3.3.0 ?

tiwarisanjay avatar Apr 21 '22 13:04 tiwarisanjay

No. It is not merged.

alexec avatar Apr 21 '22 14:04 alexec

Hey folks, I have had a change in circumstances and have lost the time to properly merge this change. Sorry about that.

lnattrass avatar Apr 21 '22 15:04 lnattrass

opened a PR to add the flag. I have not tested though

ChaosInTheCRD avatar May 10 '22 14:05 ChaosInTheCRD

@alexec @lnattrass Do you when we are planning on merging this change ?

tiwarisanjay avatar Jun 08 '22 13:06 tiwarisanjay

I think this PR needs a new owner.

alexec avatar Aug 02 '22 17:08 alexec

@alexec As someone said earlier external cert support is there in the code, I was hoping it was just needs to be exposed.

tiwarisanjay avatar Aug 05 '22 00:08 tiwarisanjay