fix(appliance): use version for image tags#63075
Conversation
f26e3a8 to
e1303ae
Compare
There was a problem hiding this comment.
Simple and stateless! This is the core of the proposal. Image metadata would need no developer input to enable upgrades.
There was a problem hiding this comment.
This is nice! I'm still a little worried, but I can't explain why so it's probably just FOC 😆
Either way if we encounter issues with this, it's an easy change 🚀
There was a problem hiding this comment.
Oh! What causes the worry? 👀
Worth hearing it out before merge!
There was a problem hiding this comment.
I should probably state what I see as the other side of the trade-off (the benefits of keeping digests), of course you're welcome to add to it 😁
Digests are another layer of validation and can be seen as a security feature. Hypothetically, if an attacker compromises a user's container registry accounts but has not gained access to their k8s API, digests prevent pulling a malicious image that has been falsely tagged as one of ours.
Subjectively, using them seems like a lot of overhead for a very hypothetical benefit. Security is not a single spectrum, it depends on our (and a user's) threat model. That's why I'm framing this as a bit hypothetical.
I vote that we just document this as a divergent behaviour of the appliance and see what early adopters think. As you note, this is a 2-way door and we can consider changing it if we get a lot of pushback. I opened an issue for this: https://linear.app/sourcegraph/issue/REL-138/document-divergences-from-helm-chart
There was a problem hiding this comment.
Ah well as you wisely saying anyway, this is a 2-way door 😁
Let's merge it and rely on the issue I split to communicate the change clearly. We can always revisit it.
Simultaneously fix a bug in the (still unreleased) appliance and change approach to container image metadata. The appliance used to maintain a map of versions to image-tags. The only version hardcoded for the still-in-development appliance was 5.3.9104, since that was the latest at the time development started. These versions were set to 5.3.2, which is incorrect. These versions were obtained from looking at the main branch of deploy-sourcegraph-helm, instead of the relevant release branch. The change in approach is to stop including the optional sha256 digest of the image in the appliance-generated config. We considered several approaches in https://linear.app/sourcegraph/issue/REL-22/spike-sketch-out-how-appliance-developers-will-handle-upgrades, please see that issue for more context. It appears simpler to statelessly determine the image tag (as being equal to the release version string) rather than add moving parts such as calling the release registry, or building a 2-phase release system to bake image checksums into the appliance once they are known post-build. These options introduce complexity and greater possibility of errors than just asking for a version. Closes https://linear.app/sourcegraph/issue/REL-135/appliance-knows-what-images-to-pull-for-a-given-release.
e1303ae to
e407ae4
Compare
There was a problem hiding this comment.
This is nice! I'm still a little worried, but I can't explain why so it's probably just FOC 😆
Either way if we encounter issues with this, it's an easy change 🚀
Simultaneously fix a bug in the (still unreleased) appliance and change approach to container image metadata. The appliance used to maintain a map of versions to image-tags. The only version hardcoded for the still-in-development appliance was 5.3.9104, since that was the latest at the time development started. These versions were set to 5.3.2, which is incorrect. These versions were obtained from looking at the main branch of deploy-sourcegraph-helm, instead of the relevant release branch.
The change in approach is to stop including the optional sha256 digest of the image in the appliance-generated config. We considered several approaches in
https://linear.app/sourcegraph/issue/REL-22/spike-sketch-out-how-appliance-developers-will-handle-upgrades, please see that issue for more context.
It appears simpler to statelessly determine the image tag (as being equal to the release version string) rather than add moving parts such as calling the release registry, or building a 2-phase release system to bake image checksums into the appliance once they are known post-build. These options introduce complexity and greater possibility of errors than just asking for a version.
Closes
https://linear.app/sourcegraph/issue/REL-135/appliance-knows-what-images-to-pull-for-a-given-release.
Test plan
Golden tests included.
Changelog