Skip to content

Conversation

@jimmidyson
Copy link
Member

This commit updates the helm registry initialization by separating out
the image used for copying the charts to the PVC and using the released
mindthegap image directly for serving the bundles. This is a small
security enhancement by using the minimal image that mindthegap already
provides.

This commit also copies a statically compiled version of cp to a scratch
container in order to us a minimal container with no package manager or
shell, again this is a minor security enhancement. This change means
that the bundles are copied to a subdirectory on the PVC as globbing
cannot be used without a shell present, whereas recursive copying works
correctly. This is a breaking change but should not affect any users at
this point (e.g. not yet included in any downstream releases).

Finally, this commit updates the helm values to use a more structured
approach. While this is a breaking change, the Helm chart is only used
to generate the clusterctl provider components YAML and as such does not
have any impact on existing users.

@jimmidyson jimmidyson requested review from dkoshkin and faiq October 31, 2024 11:56
This commit updates the helm registry initialization by separating out
the image used for copying the charts to the PVC and using the released
mindthegap image directly for serving the bundles. This is a small
security enhancement by using the minimal image that mindthegap already
provides.

This commit also copies a statically compiled version of `cp` to a scratch
container in order to us a minimal container with no package manager or
shell, again this is a minor security enhancement. This change means
that the bundles are copied to a subdirectory on the PVC as globbing
cannot be used without a shell present, whereas recursive copying works
correctly. This is a breaking change but should not affect any users at
this point (e.g. not yet included in any downstream releases).

Finally, this commit updates the helm values to use a more structured
approach. While this is a breaking change, the Helm chart is only used
to generate the clusterctl provider components YAML and as such does not
have any impact on existing users.
@jimmidyson jimmidyson force-pushed the jimmi/image-bundle-initializer branch from d728e38 to d5245bd Compare October 31, 2024 12:31
Copy link
Contributor

@dkoshkin dkoshkin left a comment

Choose a reason for hiding this comment

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

Tested manually in an upstream project:

Events:
  Type    Reason                  Age   From                     Message
  ----    ------                  ----  ----                     -------
  Normal  Scheduled               3m3s  default-scheduler        Successfully assigned caren-system/helm-repository-8475cb4899-xpj65 to dkoshkin-caren-961-1-md-0-288nf-xx7qj-5hcmh
  Normal  SuccessfulAttachVolume  99s   attachdetach-controller  AttachVolume.Attach succeeded for volume "pvc-44ab424c-8f3e-4655-aea4-35c134392a9a"
  Normal  Pulling                 71s   kubelet                  Pulling image "docker.io/dkoshkin/caren-helm-reg:v0.22.0-dev-amd64-1"
  Normal  Pulled                  68s   kubelet                  Successfully pulled image "docker.io/dkoshkin/caren-helm-reg:v0.22.0-dev-amd64-1" in 3.011s (3.011s including waiting). Image size: 2440554 bytes.
  Normal  Created                 68s   kubelet                  Created container copy-charts
  Normal  Started                 68s   kubelet                  Started container copy-charts
  Normal  Pulling                 67s   kubelet                  Pulling image "ghcr.io/mesosphere/mindthegap:v1.16.0"
  Normal  Pulled                  64s   kubelet                  Successfully pulled image "ghcr.io/mesosphere/mindthegap:v1.16.0" in 3.007s (3.007s including waiting). Image size: 24170415 bytes.
  Normal  Created                 64s   kubelet                  Created container helm-repository
  Normal  Started                 64s   kubelet                  Started container helm-repository

The mindthegap Pod started up with all charts:

Defaulted container "helm-repository" out of: helm-repository, copy-charts (init)
2024-10-31 20:00:13 INF  • Creating temporary directory...
 ✓ Creating temporary directory
2024-10-31 20:00:13 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.6.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.6.tar"
2024-10-31 20:00:13 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-10-31 20:00:13 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.9.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.9.tar"
2024-10-31 20:00:13 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-10-31 20:00:13 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.22.0-dev.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.22.0-dev.tar"
2024-10-31 20:00:13 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-10-31 20:00:13 INF  • Creating Docker registry...
 ✓ Creating Docker registry
2024-10-31 20:00:13 INF Listening on 0.0.0.0:5000

Copy link
Contributor

@dkoshkin dkoshkin left a comment

Choose a reason for hiding this comment

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

Tested upgrading from a previous version and everything is looking great.

k logs -n caren-system helm-repository-66d86975f5-b6cpx
Defaulted container "helm-repository" out of: helm-repository, copy-charts (init)
2024-11-05 18:20:22 INF  • Creating temporary directory...
 ✓ Creating temporary directory
2024-11-05 18:20:22 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.6.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.6.tar"
2024-11-05 18:20:22 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-11-05 18:20:22 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.9.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.14.9.tar"
2024-11-05 18:20:22 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-11-05 18:20:22 INF  • Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.22.0-dev.tar"...
 ✓ Unarchiving image bundle "/helm-charts/bundles/helm-charts-v0.22.0-dev.tar"
2024-11-05 18:20:22 INF  • Parsing Helm charts bundle config...
 ✓ Parsing Helm charts bundle config
2024-11-05 18:20:22 INF  • Creating Docker registry...
 ✓ Creating Docker registry
2024-11-05 18:20:22 INF Listening on 0.0.0.0:5000

@jimmidyson jimmidyson force-pushed the jimmi/image-bundle-initializer branch from d729740 to cda1a80 Compare November 6, 2024 18:52
@jimmidyson jimmidyson force-pushed the jimmi/image-bundle-initializer branch from cda1a80 to 710471c Compare November 6, 2024 19:05
@jimmidyson jimmidyson enabled auto-merge (squash) November 6, 2024 21:33
@jimmidyson jimmidyson merged commit a3adcb7 into main Nov 6, 2024
20 checks passed
@jimmidyson jimmidyson deleted the jimmi/image-bundle-initializer branch November 6, 2024 21:33
@github-actions github-actions bot mentioned this pull request Nov 6, 2024
dkoshkin pushed a commit that referenced this pull request Nov 6, 2024
🤖 I have created a release *beep* *boop*
---


## 0.22.0 (2024-11-06)

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

## What's Changed
### Exciting New Features 🎉
* feat: set default CoreDNS version by @dkoshkin in
#959
### Fixes 🔧
* fix: Use correct filename for runtime extensions component YAML by
@jimmidyson in
#960
### Other Changes
* docs: Update hugo and docsy by @jimmidyson in
#958
* refactor: Update helm registry initialization by @jimmidyson in
#961


**Full Changelog**:
v0.21.0...v0.22.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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants