deckhouse icon indicating copy to clipboard operation
deckhouse copied to clipboard

Choice "source of truth" of deckhouse registry and add d8-system/deckhouse-registry secret validation webhook

Open name212 opened this issue 4 years ago • 3 comments

Preflight Checklist

  • [X] I agree to follow the Code of Conduct that this project adheres to.
  • [X] I have searched the issue tracker for an issue that matches the one I want to file, without success.

Version

v1.29.0

Expected Behavior

Deckhouse registry for modules images and bashible should get from one source.

If set incorrect credentials for Secret d8-system/deckhouse registry, cluster should not break.

Actual Behavior

For Deckhouse modules we get Deckhouse registry (registry host and registry path) from Deckhouse deployment, but for bashible steps we get registry from d8-system/deckhouse-registry secret. It is confusing. We need one source of true for it.

If we put incorrect registry credentials to d8-system/deckhouse-registry secret, we can break cluster. Why? When d8-system/deckhouse-registry secret was changed, Deckhouse deployment will be restarted. Dechouse controller will be restarted successfully, because we write deckhouse credentials into containerd config and image will be pulled, although d8-system/deckhouse-registry is imagePullSecret and Deckouse deployment has imagePullPolicy - Always.

After restart, Deckhouse controller will change control-plane-manager module with incorrect credentials and control-plane-manager change all static pods manifests (etcd, kube-apiserver...) and cluster became not working, because kubelet will not pull images.

It is not good for single-master clusters.

Steps To Reproduce

No response

Additional Information

For second case (with deckhouse-registry secret) we can add validation web hook.

Logs

No response

name212 avatar Jan 31 '22 15:01 name212

Related with #194

EvgenySamoylov avatar Apr 19 '22 11:04 EvgenySamoylov

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 18 '22 14:07 stale[bot]

This issue has been automatically closed because it has not had activity in the last month and a half. If this issue is still valid, please ping a maintainer and ask them to check this again. Thank you for your contributions.

stale[bot] avatar Jul 28 '22 19:07 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 27 '22 02:10 stale[bot]

Second part of the issue (follow-up from the meeting):

  • [ ] remove deckhouse-registry secret mount from the deckhouse deployment
  • [ ] dependency/cr remove dependency from file /etc/registrysecret/.dockerconfigjson, figure out how to pass auth data into it (maybe set some helper)
  • [ ] in the hook deckhouse/modules/002-deckhouse/hooks/set_module_image_value.go generate currentImage from global.modulesImages.registry.base + deployment image tag instead of deployment image
  • [ ] in the hook /deckhouse/modules/002-deckhouse/hooks/images_copier.go remove parsing of the registry host from deckhouse.internal.currentReleaseImageName
  • [ ] validation webhook: check registry availability with curl on secret change

yalosev avatar Dec 06 '22 10:12 yalosev

in the hook deckhouse/modules/002-deckhouse/hooks/set_module_image_value.go generate currentImage from global.modulesImages.registry.base + deployment image tag instead of deployment image

@yalosev Why do we should do this? I see in the testcases that registry.base and what is specified in the image of the deployment are different and currentReleaseImageName gets what is in the deployment.

https://github.com/deckhouse/deckhouse/blob/2ffddb57b1cdc56e676c8f199ad019cde880adb7/modules/002-deckhouse/hooks/set_module_image_value_test.go#L65

fl64 avatar Jan 10 '23 14:01 fl64

In a meeting with @apolovov @yalosev it was decided to drop the validation webhook and add a check on the existence of the deckhouse image to the change-registry.sh script

fl64 avatar Jan 17 '23 16:01 fl64