fix(concourse): support old and new concourse auth #762

jaredstehler · 2020-05-27T19:49:07Z

Fixes concourse authentication for clusters >= v6.1.0 by detecting version and providing different implementation accordingly.

Addresses issue: spinnaker/spinnaker#5797

Fixes concourse authentication for clusters >= v6.1.0 by detecting version and providing different implementation accordingly. Addresses issue: spinnaker/spinnaker#5797

Sending basic auth header to /info endpoint causes 401 Addresses issue: spinnaker/spinnaker#5797

jervi · 2020-06-17T09:16:55Z

igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/client/ConcourseClient.java

    tokenExpiration = token.getExpiry();
    return token;
  }

+  public Response userInfo() {
+    return skyServiceV1 != null ? skyServiceV1.userInfo() : skyServiceV2.userInfo();
+  }


This feels a bit dirty to me. I'd consider creating a new SkyService interface that both V1 and V2 extends from, given that they both just contains the userInfo() method signature. Then you can have just a single skyService field, and not care about the implementation and possible null-values here.
And then do exactly the same with TokenService.

Example:

public interface SkyServiceV2 extends SkyService { @GET("/api/v1/user") @Override Response userInfo(); }

fully agree! unfortunately, the library in use here, retrofit, doesn't allow extending interfaces :(

see square/retrofit#504 (appears this may have been fixed in a recent version; the one in use here is quite old)

Oh, I wasn't aware... Yuck. 😛
Maybe put a little comment in the code with that explanation?

jervi

Thanks a lot! I think this is great 👍
I've suggested a couple of changes related to generalization of SkyService and TokenService to get rid of nullable fields in the ConcourseClient class, but apart from that, this looks good to me.

donnyyung · 2020-06-30T20:30:39Z

Would it be possible for us to cherry-pick this commit into 1.21 and 1.20 as well?

jaredstehler · 2020-07-01T16:57:25Z

@Mergifyio backport release-1.20.x

mergify · 2020-07-01T16:57:57Z

@jaredstehler is not allowed to run commands

PFacheris · 2020-07-20T15:51:21Z

Noticed the above backport attempt failed and would also like to see this backported.

@robzienert + @jervi are you the appropriate people to ping for that?

kevinawoo · 2020-07-21T18:46:37Z

FYI, we have a new backport policy that only accepts regressions. There isn't a regression that the community introduced, but an external service changed.

Since we don't have any control of external services and we're supporting 3 minor versions, if concourse decided to change their auth and it broke a prior version, then Spinnaker as a service does have a regression and we'll need to backport the fix for it.

@Mergifyio backport release-1.20.x release-1.21.x

kevinawoo · 2020-07-21T19:18:05Z

@Mergifyio backport release-1.20.x

mergifyio seems to not be working, so we made #824

kevinawoo · 2020-07-21T19:18:12Z

@Mergifyio backport release-1.21.x

mergifyio doesn't work for some reason, so we just made #823

* fix(concourse): support old and new concourse auth Fixes concourse authentication for clusters >= v6.1.0 by detecting version and providing different implementation accordingly. Addresses issue: spinnaker/spinnaker#5797 * fix(concourse): support old and new concourse auth Sending basic auth header to /info endpoint causes 401 Addresses issue: spinnaker/spinnaker#5797 Co-authored-by: Jared Stehler <jared.stehler@edgenuity.com>

* fix(concourse): support old and new concourse auth Fixes concourse authentication for clusters >= v6.1.0 by detecting version and providing different implementation accordingly. Addresses issue: spinnaker/spinnaker#5797 * fix(concourse): support old and new concourse auth Sending basic auth header to /info endpoint causes 401 Addresses issue: spinnaker/spinnaker#5797 Co-authored-by: Jared Stehler <jared.stehler@edgenuity.com> Co-authored-by: Jared Stehler <jared.stehler@gmail.com> Co-authored-by: Jared Stehler <jared.stehler@edgenuity.com>

mergify · 2020-07-22T08:08:31Z

Command backport release-1.20.x release-1.21.x: failure

No backport have been created

Backport to branch release-1.20.x failed

Cherry-pick of 97e62db has failed:

On branch mergify/bp/release-1.20.x/pr-762
Your branch is up to date with 'origin/release-1.20.x'.

You are currently cherry-picking commit 97e62db2.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

Backport to branch release-1.21.x failed

Cherry-pick of 97e62db has failed:

On branch mergify/bp/release-1.21.x/pr-762
Your branch is up to date with 'origin/release-1.21.x'.

You are currently cherry-picking commit 97e62db2.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

mergify · 2020-07-22T08:08:34Z

Command backport release-1.20.x: failure

No backport have been created

Backport to branch release-1.20.x failed

Cherry-pick of 97e62db has failed:

On branch mergify/bp/release-1.20.x/pr-762
Your branch is up to date with 'origin/release-1.20.x'.

You are currently cherry-picking commit 97e62db2.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

mergify · 2020-07-22T08:08:37Z

Command backport release-1.21.x: failure

No backport have been created

Backport to branch release-1.21.x failed

Cherry-pick of 97e62db has failed:

On branch mergify/bp/release-1.21.x/pr-762
Your branch is up to date with 'origin/release-1.21.x'.

You are currently cherry-picking commit 97e62db2.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

fix(concourse): support old and new concourse auth

66f44d0

Fixes concourse authentication for clusters >= v6.1.0 by detecting version and providing different implementation accordingly. Addresses issue: spinnaker/spinnaker#5797

jaredstehler mentioned this pull request Jun 2, 2020

Concourse 6.1.0+ breaks existing igor auth spinnaker/spinnaker#5797

Closed

gal-yardeni requested review from marchello2000 and robzienert June 2, 2020 15:32

fix(concourse): support old and new concourse auth

005425d

Sending basic auth header to /info endpoint causes 401 Addresses issue: spinnaker/spinnaker#5797

gal-yardeni requested a review from jervi June 16, 2020 23:12

jervi reviewed Jun 17, 2020

View reviewed changes

jervi suggested changes Jun 17, 2020

View reviewed changes

jervi approved these changes Jun 17, 2020

View reviewed changes

robzienert approved these changes Jun 29, 2020

View reviewed changes

robzienert added the ready to merge Approved and ready for merge label Jun 29, 2020

mergify bot added the auto merged label Jun 29, 2020

Merge branch 'master' into fix/concourse-auth

317b300

mergify bot merged commit 97e62db into spinnaker:master Jun 30, 2020

spinnakerbot added the target-release/1.22 label Jun 30, 2020

kevinawoo mentioned this pull request Jul 23, 2020

feat(cherry-pick): add 'backport' as a recongized cherry-pick command spinnaker/spinnakerbot#17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(concourse): support old and new concourse auth #762

fix(concourse): support old and new concourse auth #762

jaredstehler commented May 27, 2020

jervi Jun 17, 2020

jervi Jun 17, 2020

jaredstehler Jun 17, 2020 •

edited

Loading

jaredstehler Jun 17, 2020

jervi Jun 17, 2020

jervi left a comment •

edited

Loading

donnyyung commented Jun 30, 2020

jaredstehler commented Jul 1, 2020

mergify bot commented Jul 1, 2020

PFacheris commented Jul 20, 2020

kevinawoo commented Jul 21, 2020

kevinawoo commented Jul 21, 2020 •

edited

Loading

kevinawoo commented Jul 21, 2020 •

edited

Loading

mergify bot commented Jul 22, 2020

mergify bot commented Jul 22, 2020

mergify bot commented Jul 22, 2020

fix(concourse): support old and new concourse auth #762

fix(concourse): support old and new concourse auth #762

Conversation

jaredstehler commented May 27, 2020

jervi Jun 17, 2020

Choose a reason for hiding this comment

jervi Jun 17, 2020

Choose a reason for hiding this comment

jaredstehler Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

jaredstehler Jun 17, 2020

Choose a reason for hiding this comment

jervi Jun 17, 2020

Choose a reason for hiding this comment

jervi left a comment • edited Loading

Choose a reason for hiding this comment

donnyyung commented Jun 30, 2020

jaredstehler commented Jul 1, 2020

mergify bot commented Jul 1, 2020

PFacheris commented Jul 20, 2020

kevinawoo commented Jul 21, 2020

kevinawoo commented Jul 21, 2020 • edited Loading

kevinawoo commented Jul 21, 2020 • edited Loading

mergify bot commented Jul 22, 2020

mergify bot commented Jul 22, 2020

mergify bot commented Jul 22, 2020

jaredstehler Jun 17, 2020 •

edited

Loading

jervi left a comment •

edited

Loading

kevinawoo commented Jul 21, 2020 •

edited

Loading

kevinawoo commented Jul 21, 2020 •

edited

Loading