Skip to content

Store push secret correctly for Connect Build with Buildah #12158

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 6 commits into
base: main
Choose a base branch
from
Open

Store push secret correctly for Connect Build with Buildah #12158

wants to merge 6 commits into from
+306 −8

Conversation

@im-konge
Copy link
Member

@im-konge im-konge commented Nov 21, 2025 •
edited
Loading

Type of change

  • Bugfix

Description

This PR fixes issue with not functional push secret in case that Buildah is used in Connect Build (and the particular feature gate).
The push secret wasn't configured correctly at all, this way we will mount it to one place and reference it by REGISTRY_AUTH_FILE env variable (so we will not have to pass it through option in every Buildah command).

Fixes #12157

Checklist

  • Make sure all tests pass
  • Reference relevant issue(s) and close them after merging

@im-konge im-konge requested a review from scholzj November 21, 2025 16:28
@im-konge im-konge self-assigned this Nov 21, 2025
@im-konge im-konge added this to the 0.50.0 milestone Nov 21, 2025
scholzj
scholzj reviewed Nov 21, 2025
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

BTW: A unit test might be nice for this as well?

@im-konge
fix one test
79e51f9 
Signed-off-by: Lukas Kral <[email protected]>
@scholzj
Copy link
Member

scholzj commented Nov 21, 2025

BTW: A unit test might be nice for this as well?

I guess ideally both for the volume mount and the env var. As it seems a bit lacking the coverage.

@im-konge
Copy link
Member Author

im-konge commented Nov 21, 2025

Yeah I will add more tests for those methods. I saw that they are not covered.

@codecov
Copy link

codecov bot commented Nov 21, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.76%. Comparing base (1553dda) to head (5331a9c).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
...imzi/operator/cluster/model/KafkaConnectBuild.java 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #12158      +/-   ##
============================================
- Coverage     74.76%   74.76%   -0.01%     
- Complexity     6618     6625       +7     
============================================
  Files           377      377              
  Lines         25334    25353      +19     
  Branches       3396     3400       +4     
============================================
+ Hits          18942    18954      +12     
- Misses         5006     5011       +5     
- Partials       1386     1388       +2
Files with missing lines Coverage Δ
...imzi/operator/cluster/model/KafkaConnectBuild.java 88.20% <83.33%> (-0.31%) ⬇️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@im-konge
add unit tests for both Kaniko and Buildah methods regarding volumes,…
425d1b0 
… volumemounts, and envvars

Signed-off-by: Lukas Kral <[email protected]>
@im-konge
merge the ifs
c1a4843 
Signed-off-by: Lukas Kral <[email protected]>
@im-konge
Copy link
Member Author

im-konge commented Nov 28, 2025

@scholzj I added UTs for both Kaniko and Buildah Volumes, VolumeMounts, and EnvVars. Please have a look if it's better now or you want to test it differently. Thanks.

scholzj
scholzj reviewed Nov 28, 2025
View reviewed changes
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Also, the test seem to be failing. Not entirely sure why ...

@scholzj
Copy link
Member

scholzj commented Nov 29, 2025

/gha run pipeline=regression,fg-regression

@github-actions
Copy link

github-actions bot commented Nov 29, 2025 •
edited
Loading

⏳ System test verification started: link

The following 6 job(s) will be executed:

  • regression-brokers-and-security-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operators-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operands-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-brokers-and-security-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operators-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operands-arm64 (oracle-vm-8cpu-32gb-arm64)

Tests will start after successful build completion.

@github-actions
Copy link

github-actions bot commented Nov 29, 2025

🎉 System test verification passed: link

@scholzj
Copy link
Member

scholzj commented Nov 29, 2025

/gha run pipeline=regression-fg

@github-actions
Copy link

github-actions bot commented Nov 29, 2025 •
edited
Loading

⏳ System test verification started: link

The following 6 job(s) will be executed:

  • regression-fg-brokers-and-security-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-operators-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-operands-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-brokers-and-security-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-fg-operators-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-fg-operands-arm64 (oracle-vm-8cpu-32gb-arm64)

Tests will start after successful build completion.

@github-actions
Copy link

github-actions bot commented Nov 30, 2025

❌ System test verification failed: link

@github-actions
Copy link

github-actions bot commented Nov 30, 2025

🎉 System test verification passed: link

ppatierno
ppatierno approved these changes Dec 1, 2025
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. I left a couple of suggestions but feel free to ignore them if you don't agree ;-)

@im-konge
Paolo's comments
5331a9c 
Signed-off-by: Lukas Kral <[email protected]>
@im-konge
Copy link
Member Author

im-konge commented Dec 1, 2025

/gha run pipeline=regression,regression-fg

@github-actions
Copy link

github-actions bot commented Dec 1, 2025 •
edited
Loading

⏳ System test verification started: link

The following 12 job(s) will be executed:

  • regression-brokers-and-security-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operators-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-operands-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-brokers-and-security-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operators-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-operands-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-fg-brokers-and-security-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-operators-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-operands-amd64 (oracle-vm-8cpu-32gb-x86-64)
  • regression-fg-brokers-and-security-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-fg-operators-arm64 (oracle-vm-8cpu-32gb-arm64)
  • regression-fg-operands-arm64 (oracle-vm-8cpu-32gb-arm64)

Tests will start after successful build completion.

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

❌ System test verification failed: link

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

❌ System test verification failed: link

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

Reviewers

@ppatierno ppatierno ppatierno approved these changes

@scholzj scholzj scholzj approved these changes

Assignees

@im-konge im-konge

Labels

bug needs review

Projects

None yet

Milestone

0.50.0

Development

Successfully merging this pull request may close these issues.

Kafka Connect Build with Buildah fails to push to container registries with authentication

3 participants

@im-konge @scholzj @ppatierno