Skip to content

fix: skip kubeadm CA file when Secret doesn't have a CA #752

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.

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

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

dkoshkin
Copy link
Contributor

@dkoshkin dkoshkin commented Jun 26, 2024

What problem does this PR solve?:
Found a bug when testing with a mirror registry where it doesn't have the CA key set in the Secret.
The handler incorrectly generated a patch to add /etc/certs/... file with a Secret source and key ca.crt, however its possible that the Secret does not have that key set.
Example of an incorrect file:

  - contentFrom:
      secret:
        key: ca.crt
        name: e2e-aws-ag-cc-test-1719374309-image-registry-mirror-credentials
    path: /etc/certs/999867407951.dkr.ecr.us-west-2.amazonaws.com-e2e-aws-ag-cc-test-1719374309.pem
    permissions: "0600"

This led to CAPI failing because ca.crt did not exist in the Secret.

Which issue(s) this PR fixes:
Fixes #

How Has This Been Tested?:

The first commit contains a failing unit tests to cover this scenario.

Special notes for your reviewer:

@github-actions github-actions bot added the fix label Jun 26, 2024
@dkoshkin dkoshkin force-pushed the dkoshkin/fix-registry-wihtout-ca-key branch from aaff4bb to cb2c6de Compare June 26, 2024 18:47
Copy link
Member

@jimmidyson jimmidyson left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@github-actions github-actions bot added fix and removed fix labels Jun 26, 2024
@jimmidyson jimmidyson merged commit 4048446 into main Jun 27, 2024
20 checks passed
@jimmidyson jimmidyson deleted the dkoshkin/fix-registry-wihtout-ca-key branch June 27, 2024 13:49
@github-actions github-actions bot mentioned this pull request Jun 27, 2024
jimmidyson added a commit that referenced this pull request Jun 27, 2024
🤖 I have created a release *beep* *boop*
---


## 0.11.0 (2024-06-27)

<!-- Release notes generated using configuration in .github/release.yaml
at main -->

## What's Changed
### Exciting New Features 🎉
* feat: Configure namespace sync in helm chart by @dlipovetsky in
#726
* feat: Support CRS for local-path provisioner and add CSI e2e by
@jimmidyson in
#737
* feat: Support HelmAddon strategy for AWS EBS by @jimmidyson in
#732
* feat: Deploy snapshot-controller as separate addon by @jimmidyson in
#734
* feat: Update AWS CCM versions and add HelmAddon strategy by
@jimmidyson in
#748
### Fixes 🔧
* fix: Namespace Sync controller should list no resources when source
namespace is empty string by @dlipovetsky in
#725
* fix: Temporarily hard-code supported PC version for Nutanix CSI by
@jimmidyson in
#751
* fix: skip kubeadm CA file when Secret doesn't have a CA by @dkoshkin
in
#752
* fix: Correctly report failed deploy of ServiceLoadBalancer by
@dlipovetsky in
#759
### Other Changes
* build: Tidy up goreleaser config by @jimmidyson in
#745
* ci: Fix up image loading for lint-test-helm by @jimmidyson in
#746
* refactor: Tidy up Nutanix CSI with consistent apply strategy by
@jimmidyson in
#733
* test(e2e): Set empty env vars for Nutanix e2e vars by @jimmidyson in
#749
* refactor: Use recommended "default" function syntax in helm templates
by @dlipovetsky in
#750
* refactor: Reusable HelmAddon strategy by @jimmidyson in
#735
* test(e2e): Various e2e tests fixes by @jimmidyson in
#754
* test(e2e): Correct default helm release names for AWS CCM and EBS CSI
by @jimmidyson in
#756


**Full Changelog**:
v0.10.0...v0.11.0

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants