Repository: openebs/node-disk-manager (go)

Issue:
Enhance feature gate package to add support for dependent feature gates
**Describe the problem/challenge you have**
The current [feature](https://github.com/openebs/node-disk-manager/tree/develop/pkg/features) supports adding features, and setting the default value of features. But there can be cases like one feature can be dependent on another. This functionality should also be included. 

for example:
if A is dependent on B, and B is set to False, then A should default to the default value.  


**Describe the solution you'd like**
A new `dependentFeature` variable should be made of type `map[Feature][]Feature`, so each feature can be dependent on multiple features. Then while [parsing](https://github.com/openebs/node-disk-manager/blob/develop/pkg/features/features.go#L126) the features, the new checks should be performed and if dependencies are not met, the feature should fall back to the default value.

**UseCase:**
A new feature `UsePartTableUUID` is being introduced which is dependent on `GPTBasedUUID`. So the package should make sure that `UsePartTableUUID` can be enabled only if `GPTBasedUUID` is also enabled.



Pull request:
feat(pkg/features): added support for feature gate dependencies
**What this PR does?**:
Features can now be added that are dependent on other features. 
Attempting to set a feature flag to true if the dependency is not met will result in an error.

**Does this PR require any upgrade changes?**:
No, I don't think so.

**If the changes in this PR are manually verified, list down the scenarios covered:**:
The unit tests included cover the scenarios:
- Setting a feature flag to true when a dependency is met, and when it is unmet.
- Setting a feature flag to true when only one of two dependencies are met.
- Having multiple feature flags with the same dependencies

**Any additional information for your reviewer?** : 
_Mention if this PR is part of any design or a continuation of previous PRs_
This supports dependent feature gates for later when UsePartTableUUID will be introduced.

**Checklist:**
- [x] Fixes #651 <issue number>
- [x] PR Title follows the convention of  `<type>(<scope>): <subject>`
- [ ] Has the change log section been updated? 
- [x] Commit has unit tests
- [ ] Commit has integration tests
- [ ] (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track: 
- [ ] (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them: 


Patch:
diff --git a/pkg/features/features.go b/pkg/features/features.go
index a7574977..63dfa047 100644
--- a/pkg/features/features.go
+++ b/pkg/features/features.go
@@ -69,6 +69,8 @@ var defaultFeatureGates = map[Feature]bool{
 	ChangeDetection: false,
 }
 
+var featureDependencies = map[Feature][]Feature{}
+
 // featureFlag is a map representing the flag and its state
 type featureFlag map[Feature]bool
 
@@ -124,9 +126,38 @@ func (fg featureFlag) SetFeatureFlag(features []string) error {
 		fg[f] = isEnabled
 	}
 
+	// We make sure features are only turned on if their dependencies are met
+	// We memoize values to avoid computing the same dependency twice
+	memoizedValues := make(featureFlag)
+	for feature := range fg {
+		_ = ValidateDependencies(feature, fg, memoizedValues)
+	}
+
 	for k, v := range fg {
 		klog.Infof("Feature gate: %s, state: %s", k, util.StateStatus(v))
 	}
 
 	return nil
 }
+
+// Ensures features are disabled if their dependencies are unmet
+// Returns true if a feature is enabled after validation
+func ValidateDependencies(feature Feature, flags featureFlag, memoizedValues featureFlag) bool {
+	if value, isMemoized := memoizedValues[feature]; isMemoized {
+		return value
+	}
+	if disabled := !flags[feature]; disabled {
+		return false
+	}
+	dependencies := featureDependencies[feature]
+	for _, dependency := range dependencies {
+		missingDependency := !ValidateDependencies(dependency, flags, memoizedValues)
+		if missingDependency {
+			flags[feature] = false
+			klog.Infof("Feature %v was set to false due to missing dependency %v", feature, dependency)
+			break
+		}
+	}
+	memoizedValues[feature] = flags[feature]
+	return flags[feature]
+}


Test patch:
diff --git a/pkg/features/features_test.go b/pkg/features/features_test.go
index ef393445..35829150 100644
--- a/pkg/features/features_test.go
+++ b/pkg/features/features_test.go
@@ -142,3 +142,238 @@ func TestSetFeatureFlag(t *testing.T) {
 		})
 	}
 }
+
+func TestBasicFeatureDependencies(t *testing.T) {
+	firstFeature := Feature("FeatureGate1")
+	secondFeature := Feature("FeatureGate2")
+	featureDependsOnFirst := Feature("FeatureGateNeeds1")
+	featureDependsOnSecond := Feature("FeatureGateNeeds2")
+	featureDependsOnBoth := Feature("FeatureGateNeeds1And2")
+
+	supportedFeatures = []Feature{
+		firstFeature,
+		secondFeature,
+		featureDependsOnFirst,
+		featureDependsOnSecond,
+		featureDependsOnBoth,
+	}
+
+	featureDependencies = map[Feature][]Feature{
+		featureDependsOnFirst:  {firstFeature},
+		featureDependsOnSecond: {secondFeature},
+		featureDependsOnBoth:   {firstFeature, secondFeature},
+	}
+
+	type args struct {
+		features []string
+	}
+
+	tests := map[string]struct {
+		args args
+		want featureFlag
+	}{
+		"unmet dependency should fail": {
+			args: args{
+				features: []string{string(featureDependsOnFirst)},
+			},
+			want: map[Feature]bool{
+				featureDependsOnFirst: false,
+			},
+		},
+		"met dependency should succeed": {
+			args: args{
+				features: []string{string(firstFeature), string(featureDependsOnFirst)},
+			},
+			want: map[Feature]bool{
+				firstFeature:          true,
+				featureDependsOnFirst: true,
+			},
+		},
+		"one of two unmet dependency should fail": {
+			args: args{
+				features: []string{string(firstFeature), string(featureDependsOnBoth)},
+			},
+			want: map[Feature]bool{
+				firstFeature:         true,
+				featureDependsOnBoth: false,
+			},
+		},
+		"all dependencies met should succeed": {
+			args: args{
+				features: []string{
+					string(firstFeature),
+					string(secondFeature),
+					string(featureDependsOnBoth)},
+			},
+			want: map[Feature]bool{
+				firstFeature:         true,
+				secondFeature:        true,
+				featureDependsOnBoth: true,
+			},
+		},
+		"multiple features can have same dependencies": {
+			args: args{
+				features: []string{
+					string(firstFeature),
+					string(secondFeature),
+					string(featureDependsOnFirst),
+					string(featureDependsOnSecond),
+					string(featureDependsOnBoth)},
+			},
+			want: map[Feature]bool{
+				firstFeature:           true,
+				secondFeature:          true,
+				featureDependsOnFirst:  true,
+				featureDependsOnSecond: true,
+				featureDependsOnBoth:   true,
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			flags := make(featureFlag)
+			err := flags.SetFeatureFlag(test.args.features)
+			if err != nil {
+				t.Errorf("SetFeatureFlag() error = %v", err)
+				return
+			}
+			if !reflect.DeepEqual(flags, test.want) {
+				t.Errorf("SetFeatureFlag() got = %v, want %v", flags, test.want)
+			}
+		})
+	}
+
+}
+
+func TestNestedFeatureDependencies(t *testing.T) {
+	firstFeature := Feature("FeatureGate1")
+	secondFeature := Feature("FeatureGate2")
+	thirdFeatureNeedsBoth := Feature("FeatureGate3Needs1And2")
+	fourthFeatureNeedsThird := Feature("FeatureGate4Needs3")
+
+	supportedFeatures = []Feature{
+		firstFeature,
+		secondFeature,
+		thirdFeatureNeedsBoth,
+		fourthFeatureNeedsThird,
+	}
+
+	featureDependencies = map[Feature][]Feature{
+		thirdFeatureNeedsBoth:   {firstFeature, secondFeature},
+		fourthFeatureNeedsThird: {thirdFeatureNeedsBoth},
+	}
+
+	type args struct {
+		init   []string
+		action []string
+	}
+
+	type want struct {
+		initResults   featureFlag
+		actionResults featureFlag
+	}
+
+	tests := map[string]struct {
+		args args
+		want want
+	}{
+		"satisfied nested features should succeed": {
+			args: args{
+				init:   []string{string(firstFeature), string(secondFeature), string(thirdFeatureNeedsBoth)},
+				action: []string{string(fourthFeatureNeedsThird)},
+			},
+			want: want{
+				initResults: map[Feature]bool{
+					firstFeature:          true,
+					secondFeature:         true,
+					thirdFeatureNeedsBoth: true,
+				},
+				actionResults: map[Feature]bool{
+					firstFeature:            true,
+					secondFeature:           true,
+					thirdFeatureNeedsBoth:   true,
+					fourthFeatureNeedsThird: true,
+				},
+			},
+		},
+		"unsatisfied nested dependency should fail": {
+			args: args{
+				init:   []string{string(firstFeature)},
+				action: []string{string(thirdFeatureNeedsBoth), string(fourthFeatureNeedsThird)},
+			},
+			want: want{
+				initResults: map[Feature]bool{
+					firstFeature: true,
+				},
+				actionResults: map[Feature]bool{
+					firstFeature:            true,
+					thirdFeatureNeedsBoth:   false,
+					fourthFeatureNeedsThird: false,
+				},
+			},
+		},
+		"turning off dependency should turn off dependant": {
+			args: args{
+				init:   []string{string(firstFeature), string(secondFeature), string(thirdFeatureNeedsBoth)},
+				action: []string{string(firstFeature + "=false")},
+			},
+			want: want{
+				initResults: map[Feature]bool{
+					firstFeature:          true,
+					secondFeature:         true,
+					thirdFeatureNeedsBoth: true,
+				},
+				actionResults: map[Feature]bool{
+					firstFeature:          false,
+					secondFeature:         true,
+					thirdFeatureNeedsBoth: false,
+				},
+			},
+		},
+		"turning off dependency should turn off nested dependants": {
+			args: args{
+				init:   []string{string(firstFeature), string(secondFeature), string(thirdFeatureNeedsBoth), string(fourthFeatureNeedsThird)},
+				action: []string{string(firstFeature + "=false")},
+			},
+			want: want{
+				initResults: map[Feature]bool{
+					firstFeature:            true,
+					secondFeature:           true,
+					thirdFeatureNeedsBoth:   true,
+					fourthFeatureNeedsThird: true,
+				},
+				actionResults: map[Feature]bool{
+					firstFeature:            false,
+					secondFeature:           true,
+					thirdFeatureNeedsBoth:   false,
+					fourthFeatureNeedsThird: false,
+				},
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			flags := make(featureFlag)
+			initErr := flags.SetFeatureFlag(test.args.init)
+			if initErr != nil {
+				t.Errorf("SetFeatureFlag() error initializing test = %v", initErr)
+				return
+			}
+			if !reflect.DeepEqual(flags, test.want.initResults) {
+				t.Errorf("SetFeatureFlag() initializing test got = %v, want %v", flags, test.want)
+				return
+			}
+			err := flags.SetFeatureFlag(test.args.action)
+			if err != nil {
+				t.Errorf("SetFeatureFlag() error = %v", err)
+				return
+			}
+			if !reflect.DeepEqual(flags, test.want.actionResults) {
+				t.Errorf("SetFeatureFlag() got = %v, want %v", flags, test.want)
+			}
+		})
+	}
+
+}