Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING. Support for namespace label matching and secret event handling #161

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Kyler-W
Copy link

@Kyler-W Kyler-W commented Jan 25, 2025

The two functional goals of this PR are:

  1. To respond to secret events (recreate deleted secrets, re-sync changed secrets, sync all target secrets when a source secret changes).
  2. To support namespace label matching for target namespaces.

Many changes were made to support these two functional goals, and more changes were made to simplify the code. I categorize these changes as breaking, functional, and cleanup.

BREAKING

  1. valueFrom inside the data field will no longer designate a source secret. To use a secret as the source, a new fromSecret field (an object with required strings name and namespace and optional string array keys) is added to the CRD. A ClusterSecret will only be valid if it has one of data or fromSecret, but not both.
  2. Creator/owner checking is done through ownerReferences on secrets (instead of the CREATE_BY_ANNOTATION) now, which will break installations with REPLACE_EXISTING: false as the controller will refuse to update preexisting secrets.

Functional

  1. To support namespace label matching, two new fields were added to the CRD: matchLabels and matchedSetsJoin
    • matchLabels takes objects with string values. It looks the same as labels set on a kubernetes resource.
    • matchedSetsJoin takes a string with value "union" or "intersection" and performs that operation to join the namespace lists generated by each matchLabel and by matchNamespace.
    • The logic for both new fields is in get_ns_list and duplicated in a new secret_belongs function.
  2. Since we need to track more than just namespace creation, the handler is renamed to on_namespace_event and is triggered by kopf.on.event instead of kopf.on.create (this also prevents kopf from setting the status field on namespace resources, which seemed excessive). The handler uses the new secret_belongs function to check every clustersecret to find if any match the new namespace state and syncs, deletes, or does nothing to reach the desired state.
  3. The secret_belongs function is added to check if a single namespace matches a clustersecret. This allows us to avoid a full get_ns_list call when only one namespace or secret changes.
  4. The on_field_match_namespace handler is renamed to on_match_fields and handles events for changes to the matchNamespace, avoidNamespaces, matchLabels, and matchedSetsJoin fields.
  5. The on_field_data handler now also handles events for changes to the fromSecret field.
  6. To support secret event handling, a new on_secret_event handler is added to both re-sync secrets managed by a ClusterSecret (determined by whether an ownerReference with kind: ClusterSecret is set on the secret) and trigger a full cluster secret re-sync if the changed secret is the source for a ClusterSecret.
  7. The create_secret_metadata function now adds an ownerReference to managed secrets' metadata. This is used by the on_secret_event handler and allows us to reduce the on_delete handler as secret deletion will be handled by kubernetes on clustersecret deletion.

Cleanup

  1. The CREATE_BY_ANNOTATION is removed from secrets in favor of ownerReferences.
  2. The on_delete handler is reduced to only removing the deleted ClusterSecret from the cache, as all secrets are now deleted by kubernetes through the ownerReferences.
  3. ClusterSecrets are cluster-scoped resources. All references to ClusterSecret namespace are removed.
  4. The LAST_SYNC_ANNOTATION is removed from secrets. It seems unnecessary and complicates current vs desired state comparisons done before secret updates. If added again, the sync_secret function will need to be updated to ignore this annotation when comparing desired to current secret states.
  5. A new function secret_belongs is added to test whether a single namespace matches a ClusterSecret.
  6. A new function sync_clustersecret is added to sync all targets of a ClusterSecret.
  7. The startup_fn handler now syncs all ClusterSecrets found in the cluster on operator start. The create_fn handler no longer handles ClusterSecret resume events.
  8. A new fromSecret field is added to the ClusterSecret CRD, which can be used to designate a source secret. This allows ClusterSecret structure validation to be handled by kubernetes, and allows valueFrom to be a key in ClusterSecret data.
  9. ClusterSecret data validation is removed from the sync_secret function. With the new CRD the validations are unnecessary.
  10. The read_data_secret function is moved into the sync_secret function, the only calling location.
  11. Fixes for avoidNamespaces copied from PR 157
  12. The syncedns list is removed from both the clustersecret status and the cache. Safely managing this list when handling secret and namespace events in parallel would add complexity to the operator for minimal gain when compared to the alternative of checking the cluster state for synced secrets.
  13. A new label CREATE_BY_LABEL is added with value set to the controlling ClusterSecret's name.
  14. A new function get_child_secret_namespaces is added to get a list of synced namespaces for a clustersecret using the CREATE_BY_LABEL. This is used in place of the removed syncedns lists.

This PR is not ready to be merged. There are several TODOs and a few potential improvements.

TODO

  1. Increment version
  2. Update documentation
  3. Update tests

Potential Improvements

  1. Update CRD to limit data field to only key/value pairs with string values now that valueFrom in data is unsupported (see matchLabels definition). done 19ddde5
  2. Validate matchedSetsJoin string (ensure it's one of union or intersection) - can this be done in the CRD?
  3. Remove the cache - all comparisons could be done against cluster state instead of cached state. Removed syncedns from the cache, leaving the clustersecret body 4463328
  4. Remove syncedns from ClusterSecret status - deletion of secrets that used to match would need to be handled differently. done 4463328
  5. Logging changes - after many code changes there may be logs that no longer make sense, and there are likely new operations that would benefit from new log output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant