fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header #6139

Badbond · 2024-01-22T14:31:26Z

Ultimately, considers the WWW-Authenticate header value as a List<String> as opposed to casting it directly to a String.

According to Mozilla's WWW-Authenticate header documentation, it is possible for the server to respond with multiple challenges through multiple header values. As such, I decided not to simply fetch the first header value, but now loop over the values and handle the first to match the bearer or basic prefix. It might be time to extract this to a separate method, but I kept the diff as small as possible.

I am not too familiar with Groovy nor Spock, so feel free to apply improvements where you deem fit.

The first commit serves as a regression test that the second commit fixes.

…ate` header

dbyron-sf · 2024-01-22T16:43:58Z

Can you look into the failures please?

Badbond · 2024-01-23T09:10:48Z

Can you look into the failures please?

Based on this line, it looks like a transient issue while initializing (a) Testcontainer(s). Locally the tests pass for me, and debugging them shows that they do not execute any code path in the changed classes. Could you please retrigger the build? If it still fails, I'd be happy to take a further look. It would be nice to have some more logs of the failing tests, if you have a way of retrieving those.

dbyron-sf · 2024-01-23T16:49:21Z

Thanks much @Badbond

aRobinson-R7 · 2024-03-27T16:24:46Z

@Badbond @dbyron-sf can we get this back-ported to v1.33 release branch please?

dbyron-sf · 2024-03-27T16:26:05Z

@Mergifyio backport release-1.33.x

mergify · 2024-03-27T16:26:28Z

backport release-1.33.x

✅ Backports have been created

#6175 fix(provider/docker): Support multiple challenges from WWW-Authenticate header (backport #6139) has been created for branch release-1.33.x

…ate` header (#6139) * test(provider/docker): Add test for `WWW-Authenticate` header parsing * fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header (cherry picked from commit 2aeea5a)

…ate` header (#6139) (#6175) * test(provider/docker): Add test for `WWW-Authenticate` header parsing * fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header (cherry picked from commit 2aeea5a) Co-authored-by: Pieter Dirk Soels <[email protected]>

aRobinson-R7 · 2024-03-27T19:51:48Z

thank you @dbyron-sf

snorlaX-sleeps · 2024-04-03T16:54:58Z

also @dbyron-sf - will this end up in the next Halyard release? Error is also occurring when editing existing dockerHub accounts added

dbyron-sf · 2024-04-03T17:02:47Z

Do you mean the next version of spinnaker? Yes, it'll end up in 1.34. Not sure anyone has released a 1.33.x since we backported it.

snorlaX-sleeps · 2024-04-03T17:20:26Z

@dbyron-sf I meant specifically that the logic from this is imported into Halyard (Afaik from reading other issues)
I was just asking if these sort of patches make it into the next release of it

dbyron-sf · 2024-04-03T18:25:10Z

Halyard has its own release cycle that's separate from the rest of spinnaker. As far as I know this logic isn't relevant to halyard. Please correct me if that's not true.

Badbond · 2024-04-04T07:02:08Z

As mentioned in the ticket, it also affects Halyard. This is because it uses Clouddriver's Docker client here for performing validations. IIUC this requires Halyard to use the new Clouddriver release and issue a release as well.

However, it may be that older versions of Halyard (< 1.63.0) are not affected as they rely on an older version of Clouddriver predating this bug. I did not backtrack this all the way (yet).

dbyron-sf · 2024-04-04T15:33:21Z

OK, so as a start, let me publish new clouddriver jars (5.84.0) from the HEAD of master that include this fix. Then the next step will be to release a new version of halyard.

dbyron-sf · 2024-04-06T05:31:15Z

OK, I've published those clouddriver jars and released version 1.64 of halyard. Can you try it out please?

Badbond · 2024-04-08T08:49:17Z

Starting with upgrading Halyard (1.62.0 -> 1.64.0), it seems it can not start the application due to a cyclic dependency:

2024-04-08 08:39:28.601 ERROR 1 --- [           main] o.s.b.d.LoggingFailureAnalysisReporter   : 

***************************
APPLICATION FAILED TO START
***************************

Description:

The dependencies of some of the beans in the application context form a cycle:

   subscriptionController defined in URL [jar:file:/opt/halyard/lib/halyard-web-1.64.0.jar!/com/netflix/spinnaker/halyard/controllers/v1/SubscriptionController.class]
      ↓
   subscriptionService (field private com.netflix.spinnaker.halyard.config.services.v1.PubsubService com.netflix.spinnaker.halyard.config.services.v1.SubscriptionService.pubsubService)
      ↓
   pubsubService (field private com.netflix.spinnaker.halyard.config.services.v1.ValidateService com.netflix.spinnaker.halyard.config.services.v1.PubsubService.validateService)
      ↓
   validateService (field com.netflix.spinnaker.halyard.config.validate.v1.ValidatorCollection com.netflix.spinnaker.halyard.config.services.v1.ValidateService.validatorCollection)
      ↓
   validatorCollection (field private java.util.List com.netflix.spinnaker.halyard.config.validate.v1.ValidatorCollection.validators)
      ↓
   deploymentConfigurationValidator (field com.netflix.spinnaker.halyard.config.services.v1.VersionsService com.netflix.spinnaker.halyard.config.validate.v1.DeploymentConfigurationValidator.versionsService)
      ↓
   versionsService (field com.netflix.spinnaker.halyard.core.registry.v1.ProfileRegistry com.netflix.spinnaker.halyard.config.services.v1.VersionsService.profileRegistry)
      ↓
   profileRegistry (field com.netflix.spinnaker.halyard.core.registry.v1.GoogleProfileReader com.netflix.spinnaker.halyard.core.registry.v1.ProfileRegistry.googleProfileReader)
┌─────┐
|  googleProfileReader (field com.google.api.services.storage.Storage com.netflix.spinnaker.halyard.core.registry.v1.GoogleProfileReader.applicationDefaultGoogleStorage)
└─────┘


Action:

Relying upon circular references is discouraged and they are prohibited by default. Update your application to remove the dependency cycle between beans. As a last resort, it may be possible to break the cycle automatically by setting spring.main.allow-circular-references to true.

Badbond · 2024-04-08T12:30:51Z

As for this particular fix in Clouddriver, I was able to upgrade Spinnaker 1.31 -> 1.33 using Halyard 1.62.0 while specifying the 5.84.0-unvalidated tag as artifactId override for Clouddriver. The issue is not apparent anymore. 🎉

dbyron-sf · 2024-04-09T22:09:13Z

@Badbond I have a feeling spinnaker/halyard#2144 fixes the halyard startup issues.

dbyron-sf · 2024-04-10T03:20:51Z

@Badbond I've released version 1.65.0 of halyard. Can you try it out?

Badbond · 2024-04-12T08:36:12Z

Thank you @dbyron-sf, it is working! 👍 We were able to upgrade to Spinnaker 1.33.0 and Halyard 1.65.0 now. Unfortunately we did stumble upon some new unrelated issues while testing and upgrading, which I have filed for completion here:

Spinnaker 1.33.0 comes with changeset checksum changes spinnaker#6941
Clouddriver 5.83.0 is unable to execute certain changesets against PostgreSQL spinnaker#6942

snorlaX-sleeps · 2024-04-12T13:50:55Z

Thanks @dbyron-sf ! (Unfortunately I cannot test as our fix was just to remove the ability to access Dockerhub at all 😂)

Badbond added 2 commits January 22, 2024 15:16

test(provider/docker): Add test for WWW-Authenticate header parsing

5386e51

fix(provider/docker): Support multiple challenges from `WWW-Authentic…

7c23aba

…ate` header

dbyron-sf approved these changes Jan 23, 2024

View reviewed changes

dbyron-sf added the ready to merge Approved and ready for a merge label Jan 23, 2024

mergify bot added the auto merged Merged automatically by a bot label Jan 23, 2024

mergify bot merged commit 2aeea5a into spinnaker:master Jan 23, 2024
16 checks passed

Badbond deleted the fix-header-parsing branch January 24, 2024 08:41

mergify bot mentioned this pull request Mar 27, 2024

fix(provider/docker): Support multiple challenges from WWW-Authenticate header (backport #6139) #6175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header #6139

fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header #6139

Badbond commented Jan 22, 2024

dbyron-sf commented Jan 22, 2024

Badbond commented Jan 23, 2024

dbyron-sf commented Jan 23, 2024

aRobinson-R7 commented Mar 27, 2024

dbyron-sf commented Mar 27, 2024

mergify bot commented Mar 27, 2024 •

edited

Loading

aRobinson-R7 commented Mar 27, 2024

snorlaX-sleeps commented Apr 3, 2024

dbyron-sf commented Apr 3, 2024

snorlaX-sleeps commented Apr 3, 2024

dbyron-sf commented Apr 3, 2024

Badbond commented Apr 4, 2024

dbyron-sf commented Apr 4, 2024 •

edited

Loading

dbyron-sf commented Apr 6, 2024

Badbond commented Apr 8, 2024

Badbond commented Apr 8, 2024

dbyron-sf commented Apr 9, 2024

dbyron-sf commented Apr 10, 2024

Badbond commented Apr 12, 2024

snorlaX-sleeps commented Apr 12, 2024

fix(provider/docker): Support multiple challenges from WWW-Authenticate header #6139

fix(provider/docker): Support multiple challenges from WWW-Authenticate header #6139

Conversation

Badbond commented Jan 22, 2024

dbyron-sf commented Jan 22, 2024

Badbond commented Jan 23, 2024

dbyron-sf commented Jan 23, 2024

aRobinson-R7 commented Mar 27, 2024

dbyron-sf commented Mar 27, 2024

mergify bot commented Mar 27, 2024 • edited Loading

✅ Backports have been created

aRobinson-R7 commented Mar 27, 2024

snorlaX-sleeps commented Apr 3, 2024

dbyron-sf commented Apr 3, 2024

snorlaX-sleeps commented Apr 3, 2024

dbyron-sf commented Apr 3, 2024

Badbond commented Apr 4, 2024

dbyron-sf commented Apr 4, 2024 • edited Loading

dbyron-sf commented Apr 6, 2024

Badbond commented Apr 8, 2024

Badbond commented Apr 8, 2024

dbyron-sf commented Apr 9, 2024

dbyron-sf commented Apr 10, 2024

Badbond commented Apr 12, 2024

snorlaX-sleeps commented Apr 12, 2024

fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header #6139

fix(provider/docker): Support multiple challenges from `WWW-Authenticate` header #6139

mergify bot commented Mar 27, 2024 •

edited

Loading

dbyron-sf commented Apr 4, 2024 •

edited

Loading