Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Use repeated --credentials flag in coursier#55113

Merged
keynmol merged 2 commits into
mainfrom
coursier-credentials-passing
Jul 24, 2023
Merged

Use repeated --credentials flag in coursier#55113
keynmol merged 2 commits into
mainfrom
coursier-credentials-passing

Conversation

@keynmol

@keynmol keynmol commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Instead of the multi-line env variable we used before, which is not escaped correctly and newlines are simply removed causing credentials problems.

Test plan

To test these changes, I did the following:

  1. Add a JVM host with two repos, both pointing to localhost
{
  // Configuration for resolving from Maven repositories.
  "maven": {
    "credentials": "localhost user:password\nsourcegraph.test user2:password2",
    "repositories": ["http://localhost:8080/", "http://sourcegraph.test:8087/"],
  }
}

They have different credentials.

  1. Launch two file proxies with basic auth

    For some reason I wrote a Scala script for that, so I'll keep it here for the future: https://gist.github.com/keynmol/25268b030c95eff2f073d49763f0fefc

  2. Verify that both proxies are being hit, and files being served

CleanShot 2023-07-24 at 16 37 55@2x

For some reason I see some requests being sent without auth at first, which might be a quirk of coursier - doesn't seem to affect the results.

So overall, I believe this is working well.

Instead of the multi-line env variable we used before, which is not
escaped correctly and newlines are simply removed causing credentials
problems.
@cla-bot cla-bot Bot added the cla-signed label Jul 19, 2023
@sourcegraph-bot

sourcegraph-bot commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff d9c9fa8...dae2cfe.

Notify File(s)
@eseliger internal/extsvc/jvmpackages/coursier/coursier.go
@sashaostrikov internal/extsvc/jvmpackages/coursier/coursier.go

Comment thread internal/extsvc/jvmpackages/coursier/coursier.go

@olafurpg olafurpg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍🏻 Thank you for spotting the root issue! Feel free to merge if local testing validates the fix works as intended.

@keynmol keynmol merged commit 462e5ec into main Jul 24, 2023
@keynmol keynmol deleted the coursier-credentials-passing branch July 24, 2023 15:41
github-actions Bot pushed a commit that referenced this pull request Jul 24, 2023
Instead of the multi-line env variable we used before, which is not
escaped correctly and newlines are simply removed causing credentials
problems.

(cherry picked from commit 462e5ec)
keynmol added a commit that referenced this pull request Jul 25, 2023
BolajiOlajide added a commit that referenced this pull request Jul 25, 2023
Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
MaedahBatool pushed a commit that referenced this pull request Jul 28, 2023
Instead of the multi-line env variable we used before, which is not
escaped correctly and newlines are simply removed causing credentials
problems.
MaedahBatool pushed a commit that referenced this pull request Jul 28, 2023
Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants