[Merged by Bors] - Mark pods for expiry when secrets are no longer applicable #114
Conversation
nightkr
changed the title
|
Do we want this to be opt-in (but disabled by default), opt-out (but enabled by default), or mandatory? Originally the issue was written assuming that this would be opt-in, but thinking about it a bit more I'm leaning towards making it mandatory for now, and transitioning towards opt-out once we implement hot-updating secrets in secret-op. |
This is the flip side of stackabletech/secret-operator#114, and implements the actual deletion of the pods once they expire.
This is the flip side of stackabletech/secret-operator#114, and implements the actual deletion of the pods once they expire.
## Description This is the flip side of stackabletech/secret-operator#114, and implements the actual eviction of the pods once they expire. - [X] Time-based eviction (`restarter.stackable.tech/expires-at.{TAG}` annotation) - [ ] Dependency-based eviction (like the current STS eviction)
## Description This is the flip side of stackabletech/secret-operator#114, and implements the actual eviction of the pods once they expire. - [X] Time-based eviction (`restarter.stackable.tech/expires-at.{TAG}` annotation) - [ ] Dependency-based eviction (like the current STS eviction)
## Description This is the flip side of stackabletech/secret-operator#114, and implements the actual eviction of the pods once they expire. - [X] Time-based eviction (`restarter.stackable.tech/expires-at.{TAG}` annotation) - [ ] Dependency-based eviction (like the current STS eviction)
There was a problem hiding this comment.
Looks good overall 👍
Could you please add a Changelog entry?
I'm asking myself if having a hash as tag is a good idea of if it would be better to use the volume name as the tag because of the following reasons:
- Easier to implement
- Easier to understand for the user (descriptive names) -> They could also set up monitoring based on the descriptive Pod Labels (I'm used to having that metrics in Prometheus already)
- No hash collisions
The resulting label names should be always valid as the volume names are also enforced to the same extend as the name below AFAIK
Annotations are key/value pairs. Valid annotation keys have two segments: an optional prefix and name, separated by a slash (/). The name segment is required and must be 63 characters or less, beginning and ending with an alphanumeric character ([a-z0-9A-Z]) with dashes (-), underscores (_), dots (.), and alphanumerics between
Yeah, this is one reason for the hash IMO, since this is more of an implementation detail than a part of secret-op's public API. We can always change this to something more human-readable later on, if we want to..
On the other hand, we increase the risk of name collisions, since we can't really namespace them anymore (without the annotations becoming gargantuan, anyway).
Well, there can be valid reasons to overwrite the annotation, if we're retrying a previously failed provisioning (or reprovisioning the secret for an existing pod). If we want to tackle this more seriously then I'd rather switch to a "real" cryptographic hash (like SHA-2 or Blake2...). |
Co-authored-by: Sebastian Bernauer <[email protected]>
Co-authored-by: Sebastian Bernauer <[email protected]>
…-operator into feature/expire-pods
|
@sbernauer Added changelog and replaced fnv with truncated sha2. |
Co-authored-by: Sebastian Bernauer <[email protected]>
|
bors r+ |
|
Build failed: |
…-operator into feature/expire-pods
|
bors retry |
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
Description
Fixes #92.
autoTlspods for expiry after a set timek8sSearchpods for expiry when listed objects are modified - postponed as Expire pods whenk8sSearchsecrets are modified #118Review Checklist
Once the review is done, comment
bors r+(orbors merge) to merge. Further information