Inclusive configuration of resources to propagate

See issue kubernetes-retired#16. To allow inclusive propagation of resources an
additional `SyncornizationMode` called 'AllowPropagate' which only enables
propagation when a selector is set is added. An 'all' selector is also addded.

Tested: e2e-testing covering secrets resource in 'AllowPropagate' mode and checking
propagation when selectors are set and unset ('select', 'treeSelect', 'none', 'all').
Unit testing is also modified to account for the new 'all' selection

Signed-off-by: mzeevi <[email protected]>

Author: mzeevi

api/v1alpha2/hierarchy_types.go

@@ -37,6 +37,7 @@ const (
 	AnnotationSelector     = AnnotationPropagatePrefix + "/select"
 	AnnotationTreeSelector = AnnotationPropagatePrefix + "/treeSelect"
 	AnnotationNoneSelector = AnnotationPropagatePrefix + "/none"
+	AnnotationAllSelector  = AnnotationPropagatePrefix + "/all"
 
 	// LabelManagedByStandard will eventually replace our own managed-by annotation (we didn't know
 	// about this standard label when we invented our own).

api/v1alpha2/hnc_config.go

@@ -31,7 +31,7 @@ const (
 )
 
 // SynchronizationMode describes propagation mode of objects of the same kind.
-// The only three modes currently supported are "Propagate", "Ignore", and "Remove".
+// The only four modes currently supported are "Propagate", "AllowPropagate", "Ignore", and "Remove".
 // See detailed definition below. An unsupported mode will be treated as "ignore".
 type SynchronizationMode string
 
@@ -46,6 +46,10 @@ const (
 
 	// Remove all existing propagated copies.
 	Remove SynchronizationMode = "Remove"
+
+	// AllowPropagate allows propagation of objects from ancestors to descendants
+	// and deletes obsolete descendants only if a an annotation is set on the object
+	AllowPropagate SynchronizationMode = "AllowPropagate"
 )
 
 const (
@@ -94,7 +98,7 @@ type ResourceSpec struct {
 	// Synchronization mode of the kind. If the field is empty, it will be treated
 	// as "Propagate".
 	// +optional
-	// +kubebuilder:validation:Enum=Propagate;Ignore;Remove
+	// +kubebuilder:validation:Enum=Propagate;Ignore;Remove;AllowPropagate
 	Mode SynchronizationMode `json:"mode,omitempty"`
 }
 

config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml

@@ -64,6 +64,7 @@ spec:
                       - Propagate
                       - Ignore
                       - Remove
+                      - AllowPropagate
                       type: string
                     resource:
                       description: Resource to be configured.

internal/forest/forest.go

@@ -51,6 +51,9 @@ type TypeSyncer interface {
 	// GetMode gets the propagation mode of objects that are handled by the reconciler who implements the interface.
 	GetMode() api.SynchronizationMode
 
+	// CanPropagate returns true if Propagate mode or AllowPropagate mode is set
+	CanPropagate() bool
+
 	// GetNumPropagatedObjects returns the number of propagated objects on the apiserver.
 	GetNumPropagatedObjects() int
 }

internal/hierarchyconfig/validator.go

@@ -246,27 +246,27 @@ func (v *Validator) getConflictingObjects(newParent, ns *forest.Namespace) []str
 	if newParent == nil {
 		return nil
 	}
-	// Traverse all the types with 'Propagate' mode to find any conflicts.
+	// Traverse all the types with 'Propagate' mode or 'AllowPropogate' mode to find any conflicts.
 	conflicts := []string{}
 	for _, t := range v.Forest.GetTypeSyncers() {
-		if t.GetMode() == api.Propagate {
-			conflicts = append(conflicts, v.getConflictingObjectsOfType(t.GetGVK(), newParent, ns)...)
+		if t.CanPropagate() {
+			conflicts = append(conflicts, v.getConflictingObjectsOfType(t.GetGVK(), t.GetMode(), newParent, ns)...)
 		}
 	}
 	return conflicts
 }
 
 // getConflictingObjectsOfType returns a list of namespaced objects if there's
 // any conflict between the new ancestors and the descendants.
-func (v *Validator) getConflictingObjectsOfType(gvk schema.GroupVersionKind, newParent, ns *forest.Namespace) []string {
+func (v *Validator) getConflictingObjectsOfType(gvk schema.GroupVersionKind, mode api.SynchronizationMode, newParent, ns *forest.Namespace) []string {
 	// Get all the source objects in the new ancestors that would be propagated
 	// into the descendants.
 	newAnsSrcObjs := make(map[string]bool)
 	for _, nnm := range newParent.GetAncestorSourceNames(gvk, "") {
 		// If the user has chosen not to propagate the object to this descendant,
 		// then it should not be included in conflict checks
 		o := v.Forest.Get(nnm.Namespace).GetSourceObject(gvk, nnm.Name)
-		if ok, _ := selectors.ShouldPropagate(o, o.GetLabels()); ok {
+		if ok, _ := selectors.ShouldPropagate(o, o.GetLabels(), mode); ok {
 			newAnsSrcObjs[nnm.Name] = true
 		}
 	}

internal/hncconfig/reconciler.go

@@ -361,7 +361,7 @@ func (r *Reconciler) writeCondition(inst *api.HNCConfiguration, tp, reason, msg
 }
 
 // setTypeStatuses adds Status.Resources for types configured in the spec. Only the status of types
-// in `Propagate` and `Remove` modes will be recorded. The Status.Resources is sorted in
+// in `Propagate`, `Remove` and `AllowPropagate` modes will be recorded. The Status.Resources is sorted in
 // alphabetical order based on Group and Resource.
 func (r *Reconciler) setTypeStatuses(inst *api.HNCConfiguration) {
 	// We lock the forest here so that other reconcilers cannot modify the
@@ -394,7 +394,7 @@ func (r *Reconciler) setTypeStatuses(inst *api.HNCConfiguration) {
 		}
 
 		// Only add NumSourceObjects if we are propagating objects of this type.
-		if ts.GetMode() == api.Propagate {
+		if ts.CanPropagate() {
 			numSrc := 0
 			nms := r.Forest.GetNamespaceNames()
 			for _, nm := range nms {

internal/hncconfig/validator.go

@@ -127,14 +127,14 @@ func (v *Validator) checkForest(ts gvkSet) admission.Response {
 	v.Forest.Lock()
 	defer v.Forest.Unlock()
 
-	// Get types that are changed from other modes to "Propagate" mode.
+	// Get types that are changed from other modes to "Propagate" mode or "AllowPropagate" mode.
 	gvks := v.getNewPropagateTypes(ts)
 
 	// Check if user-created objects would be overwritten by these mode changes.
-	for gvk := range gvks {
-		conflicts := v.checkConflictsForGVK(gvk)
+	for gvk, mode := range gvks {
+		conflicts := v.checkConflictsForGVK(gvk, mode)
 		if len(conflicts) != 0 {
-			msg := fmt.Sprintf("Cannot update configuration because setting type %q to 'Propagate' mode would overwrite user-created object(s):\n", gvk)
+			msg := fmt.Sprintf("Cannot update configuration because setting type %q to 'Propagate' mode or 'AllowPropagate' mode would overwrite user-created object(s):\n", gvk)
 			msg += strings.Join(conflicts, "\n")
 			msg += "\nTo fix this, please rename or remove the conflicting objects first."
 			err := errors.New(msg)
@@ -147,17 +147,17 @@ func (v *Validator) checkForest(ts gvkSet) admission.Response {
 }
 
 // checkConflictsForGVK looks for conflicts from top down for each tree.
-func (v *Validator) checkConflictsForGVK(gvk schema.GroupVersionKind) []string {
+func (v *Validator) checkConflictsForGVK(gvk schema.GroupVersionKind, mode api.SynchronizationMode) []string {
 	conflicts := []string{}
 	for _, ns := range v.Forest.GetRoots() {
-		conflicts = append(conflicts, v.checkConflictsForTree(gvk, ancestorObjects{}, ns)...)
+		conflicts = append(conflicts, v.checkConflictsForTree(gvk, ancestorObjects{}, ns, mode)...)
 	}
 	return conflicts
 }
 
 // checkConflictsForTree check for all the gvk objects in the given namespaces, to see if they
 // will be potentially overwritten by the objects on the ancestor namespaces
-func (v *Validator) checkConflictsForTree(gvk schema.GroupVersionKind, ao ancestorObjects, ns *forest.Namespace) []string {
+func (v *Validator) checkConflictsForTree(gvk schema.GroupVersionKind, ao ancestorObjects, ns *forest.Namespace, mode api.SynchronizationMode) []string {
 	conflicts := []string{}
 	// make a local copy of the ancestorObjects so that the original copy doesn't get modified
 	objs := ao.copy()
@@ -167,7 +167,7 @@ func (v *Validator) checkConflictsForTree(gvk schema.GroupVersionKind, ao ancest
 		for _, nnm := range objs[onm] {
 			// check if the existing ns will propagate this object to the current ns
 			inst := v.Forest.Get(nnm).GetSourceObject(gvk, onm)
-			if ok, _ := selectors.ShouldPropagate(inst, ns.GetLabels()); ok {
+			if ok, _ := selectors.ShouldPropagate(inst, ns.GetLabels(), mode); ok {
 				conflicts = append(conflicts, fmt.Sprintf("  Object %q in namespace %q would overwrite the one in %q", onm, nnm, ns.Name()))
 			}
 		}
@@ -178,26 +178,29 @@ func (v *Validator) checkConflictsForTree(gvk schema.GroupVersionKind, ao ancest
 	// it's impossible to get cycles from non-root.
 	for _, cnm := range ns.ChildNames() {
 		cns := v.Forest.Get(cnm)
-		conflicts = append(conflicts, v.checkConflictsForTree(gvk, objs, cns)...)
+		conflicts = append(conflicts, v.checkConflictsForTree(gvk, objs, cns, mode)...)
 	}
 	return conflicts
 }
 
 // getNewPropagateTypes returns a set of types that are changed from other modes
-// to `Propagate` mode.
+// to `Propagate` or `AllowPropagate` mode.
 func (v *Validator) getNewPropagateTypes(ts gvkSet) gvkSet {
-	// Get all "Propagate" mode types in the new configuration.
+	// Get all "Propagate" mode and "AllowPropagate" mode types in the new configuration.
 	newPts := gvkSet{}
 	for gvk, mode := range ts {
 		if mode == api.Propagate {
 			newPts[gvk] = api.Propagate
 		}
+		if mode == api.AllowPropagate {
+			newPts[gvk] = api.AllowPropagate
+		}
 	}
 
-	// Remove all existing "Propagate" mode types in the forest (current configuration).
+	// Remove all existing "Propagate" mode and "AllowPropagate" mode types in the forest (current configuration).
 	for _, t := range v.Forest.GetTypeSyncers() {
 		_, exist := newPts[t.GetGVK()]
-		if t.GetMode() == api.Propagate && exist {
+		if t.CanPropagate() && exist {
 			delete(newPts, t.GetGVK())
 		}
 	}

internal/kubectl/configdescribe.go

@@ -37,6 +37,8 @@ var configDescribeCmd = &cobra.Command{
 				action = "Propagating"
 			case api.Remove:
 				action = "Removing"
+			case api.AllowPropagate:
+				action = "AllowPropagate"
 			default:
 				action = "Ignoring"
 			}

internal/kubectl/configset.go

@@ -27,8 +27,8 @@ import (
 )
 
 var setResourceCmd = &cobra.Command{
-	Use: fmt.Sprintf("set-resource RESOURCE [--group GROUP] [--force] --mode <%s|%s|%s>",
-		api.Propagate, api.Remove, api.Ignore),
+	Use: fmt.Sprintf("set-resource RESOURCE [--group GROUP] [--force] --mode <%s|%s|%s|%s>",
+		api.Propagate, api.Remove, api.Ignore, api.AllowPropagate),
 	Short: "Sets the HNC configuration of a specific resource",
 	Example: fmt.Sprintf("  # Set configuration of a core type\n" +
 		"  kubectl hns config set-resource secrets --mode Ignore\n\n" +
@@ -40,16 +40,17 @@ var setResourceCmd = &cobra.Command{
 		flags := cmd.Flags()
 		group, _ := flags.GetString("group")
 		modeStr, _ := flags.GetString("mode")
-		mode := api.SynchronizationMode(cases.Title(language.English).String(modeStr))
+		// Capitalize the user-input mode to match expected SynchronizationMode
+		mode := modeCapitalization(api.SynchronizationMode(cases.Title(language.English).String(modeStr)))
 		force, _ := flags.GetBool("force")
 		config := client.getHNCConfig()
 
 		exist := false
 		for i := 0; i < len(config.Spec.Resources); i++ {
 			r := &config.Spec.Resources[i]
 			if r.Group == group && r.Resource == resource {
-				if r.Mode == api.Ignore && mode == api.Propagate && !force {
-					fmt.Printf("Switching directly from 'Ignore' to 'Propagate' mode could cause existing %q objects in "+
+				if r.Mode == api.Ignore && (mode == api.Propagate || mode == api.AllowPropagate) && !force {
+					fmt.Printf("Switching directly from 'Ignore' to 'Propagate' mode or 'AllowPropagate' mode could cause existing %q objects in "+
 						"child namespaces to be overwritten by objects from ancestor namespaces.\n", resource)
 					fmt.Println("If you are sure you want to proceed with this operation, use the '--force' flag.")
 					fmt.Println("If you are not sure and would like to see what source objects would be overwritten," +
@@ -78,7 +79,14 @@ var setResourceCmd = &cobra.Command{
 
 func newSetResourceCmd() *cobra.Command {
 	setResourceCmd.Flags().String("group", "", "The group of the resource; may be omitted for core resources (or explicitly set to the empty string)")
-	setResourceCmd.Flags().String("mode", "", "The synchronization mode: one of Propagate, Remove or Ignore")
-	setResourceCmd.Flags().BoolP("force", "f", false, "Allow the synchronization mode to be changed directly from Ignore to Propagate despite the dangers of doing so")
+	setResourceCmd.Flags().String("mode", "", "The synchronization mode: one of Propagate, Remove, Ignore and AllowPropagate")
+	setResourceCmd.Flags().BoolP("force", "f", false, "Allow the synchronization mode to be changed directly from Ignore to Propagate or AllowPropagate despite the dangers of doing so")
 	return setResourceCmd
 }
+
+func modeCapitalization(mode api.SynchronizationMode) api.SynchronizationMode {
+	if mode == "Allowpropagate" {
+		return api.AllowPropagate
+	}
+	return mode
+}

internal/objects/reconciler.go

@@ -165,7 +165,7 @@ func (r *Reconciler) GetMode() api.SynchronizationMode {
 // treated as api.Ignore.
 func GetValidateMode(mode api.SynchronizationMode, log logr.Logger) api.SynchronizationMode {
 	switch mode {
-	case api.Propagate, api.Ignore, api.Remove:
+	case api.Propagate, api.Ignore, api.Remove, api.AllowPropagate:
 		return mode
 	case "":
 		log.Info("Sync mode is unset; using default 'Propagate'")
@@ -198,6 +198,11 @@ func (r *Reconciler) SetMode(ctx context.Context, log logr.Logger, mode api.Sync
 	return nil
 }
 
+// CanPropagate returns true if Propagate mode or AllowPropagate mode is set
+func (r *Reconciler) CanPropagate() bool {
+	return (r.GetMode() == api.Propagate || r.GetMode() == api.AllowPropagate)
+}
+
 // GetNumPropagatedObjects returns the number of propagated objects of the GVK handled by this object reconciler.
 func (r *Reconciler) GetNumPropagatedObjects() int {
 	r.propagatedObjectsLock.Lock()
@@ -388,11 +393,13 @@ func (r *Reconciler) shouldSyncAsPropagated(log logr.Logger, inst *unstructured.
 	}
 
 	// If there's a conflicting source in the ancestors (excluding itself) and the
-	// the type has 'Propagate' mode, the object will be overwritten.
+	// the type has 'Propagate' mode or 'AllowPropagate' mode, the object will be overwritten.
 	mode := r.Forest.GetTypeSyncer(r.GVK).GetMode()
-	if mode == api.Propagate && srcInst != nil {
-		log.Info("Conflicting object found in ancestors namespace; will overwrite this object", "conflictingAncestor", srcInst.GetNamespace())
-		return true, srcInst
+	if mode == api.Propagate || mode == api.AllowPropagate {
+		if srcInst != nil {
+			log.Info("Conflicting object found in ancestors namespace; will overwrite this object", "conflictingAncestor", srcInst.GetNamespace())
+			return true, srcInst
+		}
 	}
 
 	return false, nil
@@ -463,7 +470,7 @@ func (r *Reconciler) syncPropagated(inst, srcInst *unstructured.Unstructured) (s
 func (r *Reconciler) syncSource(log logr.Logger, src *unstructured.Unstructured) {
 	// Update or create a copy of the source object in the forest. We now store
 	// all the source objects in the forests no matter if the mode is 'Propagate'
-	// or not, because HNCConfig webhook will also check the non-'Propagate' mode
+	// or not, because HNCConfig webhook will also check the non-'Propagate' or non-'AllowPropagate' modes
 	// source objects in the forest to see if a mode change is allowed.
 	ns := r.Forest.Get(src.GetNamespace())
 
@@ -712,7 +719,7 @@ func hasPropagatedLabel(inst *unstructured.Unstructured) bool {
 // - Service Account token secrets
 func (r *Reconciler) shouldPropagateSource(log logr.Logger, inst *unstructured.Unstructured, dst string) bool {
 	nsLabels := r.Forest.Get(dst).GetLabels()
-	if ok, err := selectors.ShouldPropagate(inst, nsLabels); err != nil {
+	if ok, err := selectors.ShouldPropagate(inst, nsLabels, r.Mode); err != nil {
 		log.Error(err, "Cannot propagate")
 		r.EventRecorder.Event(inst, "Warning", api.EventCannotParseSelector, err.Error())
 		return false