fix: make upgrade

[phbelitz]

In ADR 1 we discussed the problem of Connaisseurs installation order and why we needed a bootstraping pod and the helm-hook image. This isn't necessary anymore. Now the webhook is part of the normal Connaisseur chart and won't be installed via the helm-hook image. Instead, an empty webhook configuration is applied in the normal installation phase next to the actual Connaisseur pods, which will be filled with the actual data in the post-install stage of the helm installation. This in it's own causes the problem, that everytime Connaisseur is upgraded, the webhook will have a new certificate, while the pods won't unless they are restarted. This is also fixed by taking an already existing certificate by using the helm lookup function and thus not changing the actual certificate.

fix #181, fix #165

[xopham]

Doesn't work for me. I get into a loop where new pods get created and instantly terminated again.

Some further notes:

• we should include the functionality in one of the integration tests (maybe one that already uninstalls and re-installs connaisseur)
• please update the docs
• after ci:add kube-linter #146 , we should be able to get rid of the kubectl config set-context --current --namespace $(NAMESPACE) and only reference the namespace in the helm command

[codecov-commenter]

Codecov Report

Merging #255 (b4d1ddd) into develop (5697366) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #255      +/-   ##
===========================================
+ Coverage    96.65%   96.69%   +0.03%     
===========================================
  Files           22       22              
  Lines         1077     1058      -19     
===========================================
- Hits          1041     1023      -18     
+ Misses          36       35       -1     

Impacted Files	Coverage Δ
connaisseur/alert.py	97.80% <ø> (+0.80%)	⬆️
connaisseur/flask_server.py	93.15% <100.00%> (-1.36%)	⬇️
connaisseur/util.py	95.34% <100.00%> (+1.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5697366...b4d1ddd. Read the comment docs.

[xopham]

Really awesome changes!! 🥳
A few notes:

• The 5min notice in Makefile could be removed
• switch from admissionregistration v1beta1 to v1: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#webhook-request-and-response
  • reference and close change webhook api version #153
• don't see why the certificate_webhook-conf.yaml is one file and the name is really off
• we really need an update mechanism for connaisseur policy/validator/secret/feature changes bc that will cause major confusion

[xopham]

Further required changes:

• Need to update the administration section in the docs/basics.md. Check for further docs that need changing.

[xopham]

you should mention how to upgrade now in docs/basics.md

[xopham]

Really like the improvement!
I am not sure if it works. Fake upgrade to a same version v2.2.1 worked. Downgrade to v2.1.2 did not work bc readiness probe failed as it requires the webhook. Will we have to make notes for upgrading for the release?

On top of my comments, please fix:

• there is several instances of the helm-hook remaining, e.g. in helm/values.yaml and some of the tests
• I suspect there will also still be some notes on it in the docs
• I would recommend to simply do a grep on the whole repo and check all instances
• please do not rebase and only fixup, as I can otherwise not track changes
• we should add an integration test to install from master and upgrade to develop. That will allow us to detect breaking changes as well. Opened ci: connaisseur upgrade integration test #298 for that

Should we wait with merging this for implementation of #280 bc currently it will be unintuitive that the config is not updated

[xopham]

made some comments

[phbelitz]

changed the pipeline, so the normal integration test is always used (for both release and develop pipeline), but when running in the develop pipeline, the Connaisseur image will be whitelisted, though a manual change in the pipeline

[xopham]

I think we are almost there.
Remaining changes are:

• silently failing when trying to delete the old imagepolicy CRD
• fixing the release.yaml: most beautiful solution would be to only overwrite that specific entry for the release case, but we would have to pick the entry from an array by number...whatever the case, it needs to be fixed to work

[xopham]

lgtm!

superseded

[xopham]

lgtm