Skip to content

feat: add bearer token support for http sync.go#435

Merged
joaopapereira merged 4 commits into
carvel-dev:developfrom
husira:develop
Apr 29, 2026
Merged

feat: add bearer token support for http sync.go#435
joaopapereira merged 4 commits into
carvel-dev:developfrom
husira:develop

Conversation

@husira

@husira husira commented Dec 26, 2025

Copy link
Copy Markdown
Contributor

This PR adds Bearer token authentication support to vendir sync (http) command:

In addition to existing HTTP Basic Auth (username / password), users can now authenticate using a Bearer token provided via secretRef.

Exactly one authentication method is allowed per secret:
• username + password → HTTP Basic Auth
• token → Authorization: Bearer

Mixed or incomplete credentials are rejected with clear validation errors.

We are using vendir to sync files from JFrog Artifactory. Our organization enforces authentication via access tokens instead of username/password. The authentication with JFrog Artifactory is implemented using Bearer tokens.

# use of username / password
apiVersion: v1
kind: Secret
metadata:
  name: secret-ref
data:
  username: dXNlcm5hbWU=
  password: cGFzc3dvcmQ=
---
apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
  - path: <path>
    contents:
    - path: "."
      http:
        url: https://<url>
        secretRef:
          name: secret-ref
# use of token
apiVersion: v1
kind: Secret
metadata:
  name: secret-ref
data:
  token: dG9rZW4=
---
apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
  - path: <path>
    contents:
    - path: "."
      http:
        url: https://<url>
        secretRef:
          name: secret-ref

I could successfully test the implementation with our registry (JFrog Artifactory) using username/password or a Bearer token.

Signed-off-by: Raphael Husistein <raphael.husistein@hotmail.com>
@husira husira changed the title feat: add bearer token support for sync.go feat: add bearer token support for http sync.go Dec 26, 2025

@joaopapereira joaopapereira left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello,
Sorry for the very very late reply but this PR felt out of my radar :(
Thanks for creating this PR.

I have some comments that would like for you to address in order to move forward with this PR
Thanks

Comment thread pkg/vendir/fetch/http/sync.go Outdated
Comment thread pkg/vendir/fetch/http/sync.go Outdated
Comment thread pkg/vendir/fetch/http/sync_test.go Outdated
Comment thread pkg/vendir/fetch/http/sync_test.go Outdated
@github-project-automation github-project-automation Bot moved this to In Progress in Carvel Feb 3, 2026
@husira

husira commented Feb 16, 2026

Copy link
Copy Markdown
Contributor Author

Hey @joaopapereira

Thank you for the updates!

I am currently traveling until mid-April. Therefore, I am unable to continue working on it at this time.

@patrickmx, Do you find time to take a look at this?

Copilot AI review requested due to automatic review settings April 14, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Bearer-token authentication support for vendir sync HTTP fetches via secretRef, alongside existing HTTP Basic Auth, with validation to reject mixed/incomplete credentials.

Changes:

  • Accept token in HTTP auth secrets and set Authorization: Bearer <token>.
  • Add stricter validation for basic-auth pairing (username/password) and disallow mixing with token auth.
  • Introduce HTTP auth-focused unit tests (basic, bearer, and invalid combinations).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
pkg/vendir/fetch/http/sync.go Adds bearer token support and credential validation in addAuth.
pkg/vendir/fetch/http/sync_test.go Adds unit tests covering basic auth, bearer token, and invalid credential combinations.
pkg/vendir/config/data.go Defines the token secret key constant used by HTTP auth.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/vendir/fetch/http/sync.go
Comment thread pkg/vendir/fetch/http/sync.go
Comment thread pkg/vendir/fetch/http/sync_test.go Outdated
Comment thread pkg/vendir/fetch/http/sync_test.go Outdated
husira added 2 commits April 14, 2026 17:52
….go to keep the tests consistent through the project

Signed-off-by: Raphael Husistein <raphael.husistein@hotmail.com>
Signed-off-by: Raphael Husistein <raphael.husistein@hotmail.com>
Copilot AI review requested due to automatic review settings April 14, 2026 16:24
@husira

husira commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

Hey @joaopapereira

I'm back after a longer vacation :) Thank you again for the review!

I have implemented the changes you suggested and tried to make the sync_test.go file consistent with other test files from the project.

Could you please review my changes again?

Thank you very much and I hope the PR can be merged soon.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/vendir/fetch/http/sync.go
Comment thread pkg/vendir/fetch/http/sync_test.go
Comment thread pkg/vendir/fetch/http/sync_test.go
@husira

husira commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Hello @joaopapereira

Do you have any update regarding this PR? Is there anything I can support with to help move it forward?

Thanks!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/vendir/fetch/http/sync_test.go

@joaopapereira joaopapereira left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@joaopapereira

Copy link
Copy Markdown
Member

going to merge this pr despite the failing linter because the majority of the linting failing does not make sense and we should make some rules a little bit more loose.

@joaopapereira joaopapereira merged commit d1ed24f into carvel-dev:develop Apr 29, 2026
12 of 14 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Closed in Carvel Apr 29, 2026
@husira

husira commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

hey @joaopapereira

Do you maybe have any insight into when this merge will be released with a new tagged version?

@github-actions github-actions Bot added the carvel-triage This issue has not yet been reviewed for validity label Jun 1, 2026
@joaopapereira

Copy link
Copy Markdown
Member

We usually release when we have enough changes or when folks ask for it.
I will try to get a release out in the next couple of days

@husira

husira commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the info. It would be great if a new version with these changes could be released soon, so we don't have to use the Develop branch. Thank you for your support!

@joaopapereira

Copy link
Copy Markdown
Member

@husira I did release vendir but forgot that we did not update the documentation, can you please create a PR that changes https://github.com/carvel-dev/carvel/tree/develop/site/content/vendir/docs/develop and https://github.com/carvel-dev/carvel/tree/develop/site/content/vendir/docs/v0.46.x with the new option?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

carvel-triage This issue has not yet been reviewed for validity

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants