dex icon indicating copy to clipboard operation
dex copied to clipboard

Implement Application Default Credentials for the google connector

Open ichbinfrog opened this issue 2 years ago • 2 comments

Overview

Implements workload identity to the google connector.

What this PR does / why we need it

Closes #1756 by implementing workload identity for the google connector.

This PR has been tested by:

  • Building the dex image
  • Injecting the image into the Argocd helm chart with Google Groups integration (see related doc)
  • Testing if Groups are visible within Argocd

I also wanted to add unit test but it required some changes to the Open interface to add option.ClientOptions... or interface{}....

Another similar PR exists but testing it yields an 404: Domain not found error.

Special notes for your reviewer

Does this PR introduce a user-facing change?

Add application default credentials for the google workspace connector

ichbinfrog avatar May 23 '22 17:05 ichbinfrog

@ichbinfrog sorry for the delay with this. The idea looks great! I only wonder about testing, adding a test is still desired. Besides it, lgtm.

nabokihms avatar Jul 25 '22 20:07 nabokihms

Moving this one to the v2.34 milestone, but still want to get this merged.

nabokihms avatar Jul 28 '22 12:07 nabokihms

our dex is hosting in gke, we need this feature too

jinnjwu avatar Aug 20 '22 09:08 jinnjwu

Thanks for the review @nabokihms and sorry for the delay, I've added the warning log as well as some unit tests for the logic.

Regarding the unit test implementation, I've given it a try. It did however require that the Open interface change to allow creating the Google Directory Service pointing to a local endpoint:

Open(id string, logger log.Logger, opts ...interface{}) (connector.Connector, error)

In the long term, this change would allow us to add further unit testing of the google connector implementation by mocking the API similarly to what's currently in the oauth connector.

ichbinfrog avatar Aug 20 '22 12:08 ichbinfrog

BTW, sorry for the long delay.

nabokihms avatar Sep 01 '22 12:09 nabokihms

There's no problem regarding the delay. My PR updates are also extremely delayed 😅

ichbinfrog avatar Sep 01 '22 14:09 ichbinfrog