Skip to content

Fix CI workflow to handle missing Docker Hub secrets #62

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
Open
wants to merge 52 commits into
base: mainline
Choose a base branch
from
Open

Fix CI workflow to handle missing Docker Hub secrets #62

wants to merge 52 commits into from

Conversation

hanxizh9910
Copy link
Contributor

Summary

Fixes #23.
This PR fixes the GitHub Actions CI workflow to properly handle cases where Docker Hub secrets are not set.

Changes

  • Skip Docker login, push, and artifact steps if Docker Hub secrets are missing.
  • Add a prepare job to detect secrets and control job/step execution.
  • Clean up dummy/test logic and focus CI on essential build, test, and push steps.

Testing

  • Verified that the workflow runs without secrets.
  • Verified that the workflow pushes to Docker Hub when secrets are set.

Branch Reference

Based on branch issue-23-ci-without-secrets.

@hanxizh9910 hanxizh9910 changed the title Modified ci.yml to run without fail when there is no secrets Fix CI workflow to handle missing Docker Hub secrets Sep 30, 2025
Nikhil-Manglore
Nikhil-Manglore reviewed Oct 1, 2025
View reviewed changes
Nikhil-Manglore
Nikhil-Manglore reviewed Oct 1, 2025
View reviewed changes
Nikhil-Manglore
Nikhil-Manglore reviewed Oct 1, 2025
View reviewed changes
Nikhil-Manglore
Nikhil-Manglore reviewed Oct 1, 2025
View reviewed changes
Nikhil-Manglore
Nikhil-Manglore approved these changes Oct 8, 2025
View reviewed changes
Copy link
Member

@Nikhil-Manglore Nikhil-Manglore left a comment

Choose a reason for hiding this comment

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

LGTM

sarthakaggarwal97
sarthakaggarwal97 reviewed Oct 8, 2025
View reviewed changes
Copy link
Collaborator

@sarthakaggarwal97 sarthakaggarwal97 left a comment

Choose a reason for hiding this comment

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

Thanks @hanxizh9910. Minor comments. @roshkhatri do you also wanna take a look?

@hanxizh9910
Modified ci.yml to run without fail when there is no secrets
8f6c0b8 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Removed some space and fixed some comments
e7e44dc 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Used more descriptive names for the job and id
f282639 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Added back the name for update docker description job
491dcb5 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Test: expect all the versions pusing to dockerhub when merge to mainline
768d652 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Added filters to rebuild only the changed images
f475c50 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Removed a comment
cdc2222 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Removed a duplicated comment
2d13cd4 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Added back original debugging aid
a4e87b2 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Test: check if the modified images will be built
30cd931 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Check if both images will be built
0fadad5 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Test: multiple files change and workflow will detect the modified doc…
959dd5c 
…ker files

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
revert the dummy changes
c315973 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Added a comment to check
fe5a71b 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
modified readme
c238710 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Avoid rebuilding the images if it is pushing to mainline
4973bc4 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
revmoed a dummy change
f2af212 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Test: dockerfile change, expect only the 8.1 jobs to run when push to…
b972e7d 
… mainline

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Test: multiple dockerfile changes, expect both 8.1 and 9.0 jobs to ru…
c205325 
…n when make a PR or push to mainline

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Merge pull request #28 from hanxizh9910/issue-23-clean
b9cf152 
Test again, try to push the images when merge to the mainline
@hanxizh9910
Last test, removed secrets, and expect nothing to be pushed but not f…
cf67691 
…aile

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Fix: fix the error when multiple changed files were flattened to one …
47d3007 
…line

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Merge pull request #30 from hanxizh9910/issue-23-clean
97bc89c 
Issue 23 clean
@hanxizh9910
Test: check if there is no dockerfile modified, expect: all the test …
f6923d0 
…run normally for PR and no test should run when merging to mainline

Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Merge pull request #31 from hanxizh9910/issue-23-clean
e86349e 
Test: check if there is no dockerfile modified, expect: all the test …
@hanxizh9910
Test: has secrets, expect all tests to run for PR and push to mainlin
8a6469d 
Signed-off-by: Hanxi Zhang <[email protected]>
@hanxizh9910
Merge pull request #32 from hanxizh9910/issue-23-clean
b467670 
Test: has secrets, expect all tests to run for PR and push to mainlin
@hanxizh9910
Copy link
Contributor Author

hanxizh9910 commented Oct 15, 2025 •
edited
Loading

I tested this by making dummy changes and merging them into my mainline branch
When there are secrets(i used my dockerhub account):
Screenshot 2025-10-15 at 3 08 54 PM
Pushing to dockerhub: https://hub.docker.com/repository/docker/hanxizh/valkey-bundle/general

When there is no secret:
Screenshot 2025-10-15 at 3 09 12 PM

Nikhil-Manglore
Nikhil-Manglore reviewed Oct 16, 2025
View reviewed changes
roshkhatri
roshkhatri reviewed Oct 16, 2025
View reviewed changes
roshkhatri
roshkhatri reviewed Oct 17, 2025
View reviewed changes
roshkhatri
roshkhatri reviewed Oct 18, 2025
View reviewed changes
@roshkhatri
Update 8.1/debian/Dockerfile
7ca2381 
Signed-off-by: Roshan Khatri <[email protected]>
@roshkhatri
Update 8.1/alpine/Dockerfile
7e14921 
Signed-off-by: Roshan Khatri <[email protected]>
@roshkhatri
Update 9.0/alpine/Dockerfile
fcb79c6 
Signed-off-by: Roshan Khatri <[email protected]>
@roshkhatri
Update 9.0/debian/Dockerfile
c0ce38e 
Signed-off-by: Roshan Khatri <[email protected]>
@roshkhatri
Update Dockerfile.template
97ecae0 
Signed-off-by: Roshan Khatri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sarthakaggarwal97 sarthakaggarwal97 sarthakaggarwal97 left review comments

@Nikhil-Manglore Nikhil-Manglore Nikhil-Manglore approved these changes

@roshkhatri roshkhatri roshkhatri left review comments

Assignees

@hanxizh9910 hanxizh9910

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CI fails on forks

4 participants

@hanxizh9910 @sarthakaggarwal97 @Nikhil-Manglore @roshkhatri