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

Add "User Service Account Impersonation" as an auth mode for GCP #7224

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

harrismendell
Copy link

@harrismendell harrismendell commented Sep 27, 2023

Issue: #7223

If the team would prefer a smaller PR
We could change this to only ever use applicationDefaultCredentials, in which case we could remove parts of the code that are altering ServiceAccountMode

@harrismendell harrismendell requested a review from a team as a code owner September 27, 2023 15:52
@@ -69,7 +69,7 @@ final private[spi] case class UnixPath(path: String) extends CharSequence {

def isAbsolute: Boolean = UnixPath.isAbsolute(path)

def isEmpty: Boolean = path.isEmpty
override def isEmpty: Boolean = path.isEmpty
Copy link
Author

Choose a reason for hiding this comment

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

Compiler was breaking without this, not related to PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what is your Java version?

I believe Cromwell is usually compiled with Java 11, and CharSequence::isEmpty is "Since: 15"

This override will need to be fixed, but maybe not right now?

Btw, I use SDKMAN! to keep all my Java SDKs under control. The JDK referenced by Cromwell's config isn't available for my M2 mac. I've just changed it on my machine to a newer 11 Temurin release.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! That was the issue. Have you had a moment to review the PR otherwise?

Copy link
Contributor

Choose a reason for hiding this comment

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

@harrismendell

Have you had a moment to review the PR otherwise?

I've glanced at it, but I'm not an official committer at the moment. I've got multiple of own PRs in the queue to get reviewed 😅

In the meantime I've just coalesced my commits into a fork and just compile and run that melange to keep moving.

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.

2 participants