Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

GCP Linking #195

Merged
merged 22 commits into from
Jul 3, 2020
Merged

GCP Linking #195

merged 22 commits into from
Jul 3, 2020

Conversation

jpcoenen
Copy link
Member

No description provided.

This allows you to set up a local listener that deals with the OAuth2 authorization callback.
This will be used to verify the existence of a link between the GCP project and the namespace.
Behaviour of ValidateGCPServiceAccountEmail was changed to only accept user-managed service accounts and  ProjectIDFromGCPEmail did not yet have a test.
@jpcoenen jpcoenen changed the base branch from master to develop June 23, 2020 13:03
Copy link
Member

@SimonBarendse SimonBarendse left a comment

Choose a reason for hiding this comment

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

I've asked a few questions in separate comments. Except from some small changes that might follow from that or some changes that might follow from my server-side review, I think this is good to go.

However, if we're including this, I think this code would benefit from some extra godoc. This will make it easier to be used and maintain.

internals/api/idp_link.go Show resolved Hide resolved
pkg/oauthorizer/callback_handler.go Outdated Show resolved Hide resolved
pkg/secrethub/idp_link.go Outdated Show resolved Hide resolved
By also return the required link type, the function can be more generally used to check the required preconditions.
No longer mixing different channels, which made it very unclear what happened. By buffering the channels and doing best-effort sending to these channels, we prevent goroutines infinitely hanging on sending on a channel. It is possible that a channel ends up with an unread message after WaitForAuthorizationCode has returned, but that will be cleaned up by the GC.
Copy link
Member

@SimonBarendse SimonBarendse left a comment

Choose a reason for hiding this comment

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

Sweet!

Only thing left is deciding whether or not to already return an iterator.

@jpcoenen jpcoenen marked this pull request as ready for review July 2, 2020 13:41
This to avoid confusion with the RedirectURL used in the OAuth spec.
@SimonBarendse SimonBarendse merged commit 9f5d3de into develop Jul 3, 2020
@SimonBarendse SimonBarendse deleted the feature/gcp-linking branch July 3, 2020 09:20
@jpcoenen jpcoenen mentioned this pull request Jul 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants