Conversation
Racer159
commented
Apr 29, 2025
- One-line PR description: Enhance Zarf's State management and tracking of resources/packages/components.
- Issue link: Zarf Enhanced State Management #26
- Other comments:
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
|
Updated this proposal to account for where Zarf is today and looking to address concerns from zarf-dev/zarf#2992 (zarf-dev/zarf#4211) / zarf-dev/zarf#4969 / zarf-dev/zarf#4182 (zarf-dev/zarf#5007) |
Signed-off-by: Wayne Starr <me@racer159.com>
AustinAbro321
left a comment
There was a problem hiding this comment.
Excited to see this develop, I think there are a lot of value adds. Some comments around UX and potential drawbacks.
brandtkeller
left a comment
There was a problem hiding this comment.
leaving one thought as I'm reading through
jasonwashburn
left a comment
There was a problem hiding this comment.
For SDK applications like the Tofu provider, I think we should consider persisting the actual successful deploy configuration itself in addition to its digest. ConfigDigest can tell us whether two configurations differ, but it cannot tell us what deployment-time configuration and values are currently represented by cluster state. Without the config itself, the tofu provider cant accurately import an existing package and hydrate its resource state or detect drift/update tofu state to show an accurate plan without making some assumptions.
I suggest adding a single top-level DeployConfig and matching ConfigDigest to DeployedPackage which represents the latest successful deployment. Failed or cancelled deploy attempts should remain visible through PackageEvent.ConfigDigest, but probably shouldn't replace the config representing the package's current installed state.
Optionally we could store each event's full config alongside PackageEvent.ConfigDigest, but I can't currently think of any immediately useful need for consuming that history...so it's probably not worth the additional Secret growth.
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
| - Redeploy (when the source is online - e.g. https/oci) | ||
| - Report deploy state / progress | ||
| - Report remove state / progress | ||
| - Resume multi-package deployment (identified by package digest + config digest) |
There was a problem hiding this comment.
I think the requirements laid out can be used to accomplish this goal but I have concerns about including it as a goal explicitly. Primarily on the basis of the Zarf project focusing on package primitives and the scope of single package operations - and enabling other tooling to build more complex handling/orchestration.
There was a problem hiding this comment.
changed it to just focus on identifying if this is the same package deployment instead of having the multi package wording.
Signed-off-by: Wayne Starr <me@racer159.com>
brandtkeller
left a comment
There was a problem hiding this comment.
lgtm - added a thought after giving this another pass with RBAC considerations but otherwise believe it to be well scoped.
|
|
||
| ##### Image reconciliation | ||
|
|
||
| When `images` is selected, Zarf diffs each changed or removed component's previously deployed `Images` against the new component's `Images` (both plain `[]string` image references on `v1alpha1.ZarfComponent`). An image present in the old set but not the new one is a pruning candidate only if no other deployed package still needs it: Zarf calls `Cluster.GetDeployedZarfPackages` to list every `DeployedPackage` secret in the cluster and checks whether the candidate image appears in any other package's component images. If nothing else references it, both tags Zarf pushes for that image are removed from the internal registry - the plain tag and the CRC-32-suffixed tag the Zarf agent uses for transparent redirection (see `images/push.go`). If another package still references it, both tags are left alone. |
There was a problem hiding this comment.
This idea is still sitting in a draft locally - but I want to at least callout the change in potential RBAC scope here. charts and components are scoped to a single package - where pruning images is going to require privileges to read all state secrets.
I haven't amended the namespace ZEP with this context yet so I wouldn't block on it - but I am trying to look for patterns we may want to start to separate from expectations.
There was a problem hiding this comment.
Hmm - how were we thinking of solving that for deploys (since they would need those push secrets)? Possibly we could use the same/similar mechanism?
There was a problem hiding this comment.
The push secrets are currently the biggest blocker but they are a read activity and not a write activity (whereas state is a write activity).
So I think there are paths to reduce required scope but I wanted to point at this image prune capability as much larger scope than the other pruning workflows. If it is a strict requirement then let's proceed - but I do want to reasonably challenge whether image pruning is an admin persona activity or an individual deployer activity. If we continue to blend the two too tightly then we may have to not try and serve the latter persona and stick to someone who has elevated privileges. Not inherently bad or different from the expectations today to be fair - I am just optimistic about broader support for multi-tenancy zarf use.
There was a problem hiding this comment.
Hmm - this also wouldn't write state though unless I am missing something? The push credentials would need to be read from state (as they would in a deploy) and then those credentials could prune images from the registry - the only new thing I see here would be that now those push credentials/state are read on a package remove vs only a package deploy.
There was a problem hiding this comment.
My explanation here is quite vague; primary point being that as it is called out I see a difference in what RBAC scope pruning requires (and that may be okay). One of my current side-quests is to see if there are potential models for multi-tenant isolation of different personas. IE the init persona (admin), the infrastructure persona (multi-namespace/cluster scope), and the application persona (single namespace).
A chart or component prune is isolated entirely to a single package which requires only the need to read a single package. If you have the required permissions to deploy then you also likely have enough to prune.
I'd like to explore amending the namespace override ZEP to writing the secret in the overridden namespace and therefor creating a model where you might need specific read permissions from the Zarf namespace, but no write permissions outside of the isolated namespace.
In this model we'd hit the constraint where you can't prune because you can read the isolated state secrets in other namespaces. This also may be okay - maybe we just error image pruning if read access to all namespaces isn't available.
Not meaning to hijack this proposal with theoretical features - but I do think we've been bitten by bloating certain workflows with everything when distinct-but-multi-step workflows have reduced scope and boundaries. IE pruning a package deployment is very much part of deploy. Pruning images feels more attuned to an admin operation (IMO).
I'm good with accepting this as is if we believe this is more intuitive and a necessary capability.