Skip to content

bk: use explicit docker login #5767

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 29, 2025
Merged

bk: use explicit docker login #5767

merged 5 commits into from
Oct 29, 2025

Conversation

@v1v
Copy link
Member

@v1v v1v commented Oct 24, 2025 •
edited
Loading

What is the problem this PR solves?

Help with fetching internal containers or from dockerhub without being affected by the rate limit or an invalid login in the VM images.

How to test this PR locally

In the CI, I ran manually:

image image

Design Checklist

  • I have ensured my design is stateless and will work when multiple fleet-server instances are behind a load balancer.
  • I have or intend to scale test my changes, ensuring it will work reliably with 100K+ agents connected.
  • I have included fail safe mechanisms to limit the load on fleet-server: rate limiting, circuit breakers, caching, load shedding, etc.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool

Related issues

#5455 moved to a golang docker image

v1v added 3 commits October 24, 2025 10:26
@v1v
bk: use explicit docker login
8bfba0c 
help with fetching internal containers or from docker hub

#5455 moved to a golang docker image

help with issues with the rate limit

help with configuring at runtime the docker login instead of using the cached one that could be invalid
@v1v
use our shared secret
2f62bff
@v1v
fips uses docker elastic registry
294e93c
@v1v v1v added the backport-active-all Automated backport with mergify to all the active branches label Oct 24, 2025
@prodsecmachine
Copy link

prodsecmachine commented Oct 24, 2025 •
edited
Loading

✅ Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
✅ Licenses 0 0 0 0 0 issues
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

v1v
v1v commented Oct 24, 2025
View reviewed changes
v1v
v1v commented Oct 24, 2025
View reviewed changes
v1v
v1v commented Oct 24, 2025
View reviewed changes
v1v
v1v commented Oct 24, 2025
View reviewed changes
v1v
v1v commented Oct 24, 2025
View reviewed changes
v1v
v1v commented Oct 24, 2025
View reviewed changes
@v1v v1v marked this pull request as ready for review October 24, 2025 13:09
@v1v v1v requested a review from a team as a code owner October 24, 2025 13:09
@v1v v1v requested a review from a team October 24, 2025 13:10
@ebeahan ebeahan requested a review from ycombinator October 24, 2025 18:49
@ebeahan
Copy link
Member

ebeahan commented Oct 24, 2025

Added @ycombinator as a reviewer. @ycombinator - can you help confirm the FIPS needing both registries?

pkoutsovasilis
pkoutsovasilis previously approved these changes Oct 27, 2025
View reviewed changes
Copy link
Contributor

@pkoutsovasilis pkoutsovasilis left a comment

Choose a reason for hiding this comment

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

Aside the FIPS requirement, where @ycombinator can chime in, the other changes LGTM

fr4nc1sc0-r4m0n
fr4nc1sc0-r4m0n previously approved these changes Oct 27, 2025
View reviewed changes
Copy link
Contributor

@fr4nc1sc0-r4m0n fr4nc1sc0-r4m0n left a comment

Choose a reason for hiding this comment

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

LGTM

@ycombinator
Copy link
Contributor

@v1v I'm trying to understand why the FIPS packaging steps in the BK pipeline need both Docker registries. Specifically, I'm trying to understand why the DockerHub registry is needed. If I look at the FROM instructions in https://github.com/elastic/fleet-server/blob/main/Dockerfile.fips, they're all pulling from the Elastic Docker registry. So, unless I missed something, I don't think we need to login to DockerHub for the FIPS packaging steps?

v1v
v1v commented Oct 28, 2025
View reviewed changes
@v1v v1v dismissed stale reviews from fr4nc1sc0-r4m0n and pkoutsovasilis via 9002915 October 28, 2025 22:04
@v1v v1v requested a review from fr4nc1sc0-r4m0n October 28, 2025 22:06
fr4nc1sc0-r4m0n
fr4nc1sc0-r4m0n approved these changes Oct 29, 2025
View reviewed changes
Copy link
Contributor

@fr4nc1sc0-r4m0n fr4nc1sc0-r4m0n left a comment

Choose a reason for hiding this comment

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

LGTM

@v1v v1v enabled auto-merge (squash) October 29, 2025 09:04
@v1v v1v disabled auto-merge October 29, 2025 14:35
@v1v v1v merged commit fe9dfc4 into main Oct 29, 2025
11 checks passed
@v1v v1v deleted the feature/use-vm-instead-of-core branch October 29, 2025 14:35
@github-actions
Copy link
Contributor

@Mergifyio backport 8.19 9.1 9.2

@mergify
Copy link
Contributor

mergify bot commented Oct 29, 2025 •
edited
Loading

backport 8.19 9.1 9.2

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 29, 2025
@v1v @mergify
bk: use explicit docker login (#5767)
5eba6f4 
(cherry picked from commit fe9dfc4)
mergify bot pushed a commit that referenced this pull request Oct 29, 2025
@v1v @mergify
bk: use explicit docker login (#5767)
14c9afa 
(cherry picked from commit fe9dfc4)
mergify bot pushed a commit that referenced this pull request Oct 29, 2025
@v1v @mergify
bk: use explicit docker login (#5767)
09cefef 
(cherry picked from commit fe9dfc4)
This was referenced Oct 29, 2025
Merged
Merged
Merged
v1v pushed a commit that referenced this pull request Oct 29, 2025
@mergify
bk: use explicit docker login (#5767) (#5785)
c904b71
v1v pushed a commit that referenced this pull request Oct 29, 2025
@mergify
bk: use explicit docker login (#5767) (#5786)
499d5ca
v1v pushed a commit that referenced this pull request Oct 29, 2025
@mergify
bk: use explicit docker login (#5767) (#5787)
73309d2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ycombinator ycombinator ycombinator approved these changes

@fr4nc1sc0-r4m0n fr4nc1sc0-r4m0n fr4nc1sc0-r4m0n approved these changes

@pkoutsovasilis pkoutsovasilis pkoutsovasilis left review comments

@blakerouse blakerouse Awaiting requested review from blakerouse blakerouse is a code owner automatically assigned from elastic/elastic-agent-control-plane

Assignees

No one assigned

Labels

backport-active-all Automated backport with mergify to all the active branches skip-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@v1v @prodsecmachine @ebeahan @ycombinator @pkoutsovasilis @fr4nc1sc0-r4m0n