fix: ensure unique condition.ValidationRule names across different rule types (#377)

## Issue
Resolves
#376

## Description
This PR ensures that the validation rule type is a part of the
validation results rule name.

Author: ahmad-ibra

build

@@ -42,7 +42,7 @@ func TestValidate(t *testing.T) {
 					},
 				}),
 			},
-			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-cluster-dc0-c0","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
+			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vsphere-privileges-cluster-dc0-c0","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
 		},
 		{
 			name: "Cluster_Fail",
@@ -59,7 +59,7 @@ func TestValidate(t *testing.T) {
 					},
 				}),
 			},
-			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-cluster-dc0-c0","message":"One or more required privileges was not found, or a condition was not met for account: admin@vsphere.local","failures":["user: admin@vsphere.local does not have privilege: Nonexistent on entity type: Cluster with name: DC0_C0"],"status":"False","lastValidationTime":null},"State":"Failed"}],"ValidationRuleErrors":[null]}`,
+			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vsphere-privileges-cluster-dc0-c0","message":"One or more required privileges was not found, or a condition was not met for account: admin@vsphere.local","failures":["user: admin@vsphere.local does not have privilege: Nonexistent on entity type: Cluster with name: DC0_C0"],"status":"False","lastValidationTime":null},"State":"Failed"}],"ValidationRuleErrors":[null]}`,
 		},
 		{
 			name: "Root_Pass",
@@ -76,7 +76,7 @@ func TestValidate(t *testing.T) {
 					},
 				}),
 			},
-			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vcenter-root","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
+			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vsphere-privileges-vcenter-root","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
 		},
 		{
 			name: "Datastore_Pass",
@@ -93,7 +93,7 @@ func TestValidate(t *testing.T) {
 					},
 				}),
 			},
-			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-datastore-localds-0","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
+			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vsphere-privileges-datastore-localds-0","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
 		},
 		{
 			name: "Network_Pass",
@@ -110,7 +110,7 @@ func TestValidate(t *testing.T) {
 					},
 				}),
 			},
-			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-network-vm-network","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
+			expected: `{"ValidationRuleResults":[{"Condition":{"validationType":"vsphere-privileges","validationRule":"validation-vsphere-privileges-network-vm-network","message":"All required vsphere-privileges permissions were found for account: admin@vsphere.local","status":"True","lastValidationTime":null},"State":"Succeeded"}],"ValidationRuleErrors":[null]}`,
 		},
 		// DistributedVirtualSwitch not yet supported in govmomi
 		// {

pkg/validate/validate_test.go

@@ -52,10 +52,11 @@ type resourceRequirement struct {
 	DiskSpace resource.Quantity
 }
 
-func buildValidationResult(rule v1alpha1.ComputeResourceRule, validationType string) *types.ValidationRuleResult {
+func buildValidationResult(rule v1alpha1.ComputeResourceRule) *types.ValidationRuleResult {
 	state := vapi.ValidationSucceeded
+	validationType := constants.ValidationTypeComputeResources
 
-	validationRule := fmt.Sprintf("%s-%s-%s", vapiconstants.ValidationRulePrefix, rule.Scope, rule.EntityName)
+	validationRule := fmt.Sprintf("%s-%s-%s-%s", vapiconstants.ValidationRulePrefix, validationType, rule.Scope, rule.EntityName)
 
 	latestCondition := vapi.DefaultValidationCondition()
 	latestCondition.Message = "All required compute resources were satisfied"
@@ -109,7 +110,7 @@ type Usage struct {
 // ReconcileComputeResourceValidationRule reconciles the compute resource rule
 func (c *ValidationService) ReconcileComputeResourceValidationRule(rule v1alpha1.ComputeResourceRule, finder *find.Finder, driver *vsphere.VCenterDriver, seenScopes map[string]bool) (*types.ValidationRuleResult, error) {
 
-	vr := buildValidationResult(rule, constants.ValidationTypeComputeResources)
+	vr := buildValidationResult(rule)
 
 	key, err := GetScopeKey(rule)
 	if err != nil {

pkg/validators/computeresources/computeresources.go

@@ -68,7 +68,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-compute-resources-cluster-dc0-c0",
 				Message:        "All required compute resources were satisfied",
 				Details:        []string{},
 				Failures:       nil,
@@ -103,7 +103,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-compute-resources-cluster-dc0-c0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: false, Memory available: true, Storage available: true"},
@@ -138,7 +138,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-compute-resources-cluster-dc0-c0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: false, Storage available: true"},
@@ -173,7 +173,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-compute-resources-cluster-dc0-c0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: true, Storage available: false"},
@@ -207,7 +207,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-esxi-host-dc0-c0-h0",
+				ValidationRule: "validation-vsphere-compute-resources-esxi-host-dc0-c0-h0",
 				Message:        "All required compute resources were satisfied",
 				Details:        []string{},
 				Failures:       nil,
@@ -241,7 +241,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-esxi-host-dc0-c0-h0",
+				ValidationRule: "validation-vsphere-compute-resources-esxi-host-dc0-c0-h0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: false, Memory available: true, Storage available: true"},
@@ -275,7 +275,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-esxi-host-dc0-c0-h0",
+				ValidationRule: "validation-vsphere-compute-resources-esxi-host-dc0-c0-h0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: false, Storage available: true"},
@@ -309,7 +309,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-esxi-host-dc0-c0-h0",
+				ValidationRule: "validation-vsphere-compute-resources-esxi-host-dc0-c0-h0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: true, Storage available: false"},
@@ -344,7 +344,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-resource-pool-dc0-c0-rp0",
+				ValidationRule: "validation-vsphere-compute-resources-resource-pool-dc0-c0-rp0",
 				Message:        "All required compute resources were satisfied",
 				Details:        []string{},
 				Failures:       nil,
@@ -379,7 +379,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-resource-pool-dc0-c0-rp0",
+				ValidationRule: "validation-vsphere-compute-resources-resource-pool-dc0-c0-rp0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: false, Memory available: true, Storage available: true"},
@@ -414,7 +414,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-resource-pool-dc0-c0-rp0",
+				ValidationRule: "validation-vsphere-compute-resources-resource-pool-dc0-c0-rp0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: false, Storage available: true"},
@@ -449,7 +449,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-resource-pool-dc0-c0-rp0",
+				ValidationRule: "validation-vsphere-compute-resources-resource-pool-dc0-c0-rp0",
 				Message:        "One or more resource requirements were not satisfied",
 				Details:        []string{},
 				Failures:       []string{"Not enough resources available. CPU available: true, Memory available: true, Storage available: false"},
@@ -484,7 +484,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-resource-pool-dc0-c1-rp0",
+				ValidationRule: "validation-vsphere-compute-resources-resource-pool-dc0-c1-rp0",
 				Message:        "Rule for scope already processed",
 				Details:        []string{},
 				Failures:       []string{"Rule for scope resource-pool-dc0-c1 already processed"},
@@ -518,7 +518,7 @@ func TestReconcileComputeResourceValidationRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-compute-resources",
-				ValidationRule: "validation-cluster-dc0-c1",
+				ValidationRule: "validation-vsphere-compute-resources-cluster-dc0-c1",
 				Message:        "Rule for scope already processed",
 				Details:        []string{},
 				Failures:       []string{"Rule for scope cluster-dc0-c1 already processed"},

pkg/validators/computeresources/computeresources_test.go

@@ -34,10 +34,11 @@ func NewValidationService(log logr.Logger, driver *vsphere.VCenterDriver, datace
 	}
 }
 
-func buildValidationResult(rule v1alpha1.NTPValidationRule, validationType string) *types.ValidationRuleResult {
+func buildValidationResult(rule v1alpha1.NTPValidationRule) *types.ValidationRuleResult {
 	state := vapi.ValidationSucceeded
+	validationType := constants.ValidationTypeNTP
 
-	validationRule := fmt.Sprintf("%s-%s", vapiconstants.ValidationRulePrefix, rule.Name())
+	validationRule := fmt.Sprintf("%s-%s-%s", vapiconstants.ValidationRulePrefix, validationType, rule.Name())
 
 	latestCondition := vapi.DefaultValidationCondition()
 	latestCondition.Message = "All required NTP rules were satisfied"
@@ -50,7 +51,7 @@ func buildValidationResult(rule v1alpha1.NTPValidationRule, validationType strin
 // ReconcileNTPRule reconciles the NTP rule
 func (n *ValidationService) ReconcileNTPRule(rule v1alpha1.NTPValidationRule, finder *find.Finder) (*types.ValidationRuleResult, error) {
 	var err error
-	vr := buildValidationResult(rule, constants.ValidationTypeNTP)
+	vr := buildValidationResult(rule)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

pkg/validators/ntp/ntp.go

@@ -43,7 +43,7 @@ func NewPrivilegeValidationService(log logr.Logger, driver *vsphere.VCenterDrive
 // ReconcilePrivilegeRule reconciles a privilege rule
 func (s *PrivilegeValidationService) ReconcilePrivilegeRule(rule v1alpha1.PrivilegeValidationRule, finder *find.Finder) (*types.ValidationRuleResult, error) {
 	var err error
-	vr := s.buildPrivilegeValidationResult(rule)
+	vr := buildValidationResult(rule, s.username)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -59,18 +59,19 @@ func (s *PrivilegeValidationService) ReconcilePrivilegeRule(rule v1alpha1.Privil
 	return vr, err
 }
 
-func (s *PrivilegeValidationService) buildPrivilegeValidationResult(rule v1alpha1.PrivilegeValidationRule) *types.ValidationRuleResult {
+func buildValidationResult(rule v1alpha1.PrivilegeValidationRule, username string) *types.ValidationRuleResult {
 	state := vapi.ValidationSucceeded
+	validationType := constants.ValidationTypePrivileges
 
-	validationRule := fmt.Sprintf("%s-%s", vapiconstants.ValidationRulePrefix, rule.EntityType)
+	validationRule := fmt.Sprintf("%s-%s-%s", vapiconstants.ValidationRulePrefix, validationType, rule.EntityType)
 	if rule.EntityName != "" {
 		validationRule = fmt.Sprintf("%s-%s", validationRule, rule.EntityName)
 	}
 
 	latestCondition := vapi.DefaultValidationCondition()
-	latestCondition.Message = fmt.Sprintf("All required %s permissions were found for account: %s", constants.ValidationTypePrivileges, s.username)
+	latestCondition.Message = fmt.Sprintf("All required %s permissions were found for account: %s", constants.ValidationTypePrivileges, username)
 	latestCondition.ValidationRule = util.Sanitize(validationRule)
-	latestCondition.ValidationType = constants.ValidationTypePrivileges
+	latestCondition.ValidationType = validationType
 
 	return &types.ValidationRuleResult{Condition: &latestCondition, State: &state}
 }

pkg/validators/privileges/privilege.go

@@ -73,7 +73,7 @@ func TestPrivilegeValidationService_ReconcilePrivilegeRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-privileges",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-privileges-cluster-dc0-c0",
 				Message:        fmt.Sprintf("All required vsphere-privileges permissions were found for account: %s", username),
 				Details:        []string{},
 				Failures:       []string{},
@@ -97,7 +97,7 @@ func TestPrivilegeValidationService_ReconcilePrivilegeRule(t *testing.T) {
 			},
 			expectedResult: types.ValidationRuleResult{Condition: &vapi.ValidationCondition{
 				ValidationType: "vsphere-privileges",
-				ValidationRule: "validation-cluster-dc0-c0",
+				ValidationRule: "validation-vsphere-privileges-cluster-dc0-c0",
 				Message:        fmt.Sprintf("One or more required privileges was not found, or a condition was not met for account: %s", username),
 				Details:        []string{},
 				Failures:       []string{"user: admin2@vsphere.local does not have privilege: VirtualMachine.Config.MagicCarpet on entity type: Cluster with name: DC0_C0"},

pkg/validators/privileges/privilege_test.go

@@ -56,7 +56,7 @@ func NewValidationService(log logr.Logger) *ValidationService {
 
 // ReconcileTagRules reconciles the tag rules
 func (s *ValidationService) ReconcileTagRules(tagsManager *tags.Manager, finder *find.Finder, driver *vsphere.VCenterDriver, rule v1alpha1.TagValidationRule) (*vapitypes.ValidationRuleResult, error) {
-	vr := buildValidationResult(rule, constants.ValidationTypeTag)
+	vr := buildValidationResult(rule)
 
 	valid, err := tagIsValid(tagsManager, finder, driver.Datacenter, rule)
 	if !valid {
@@ -71,10 +71,11 @@ func (s *ValidationService) ReconcileTagRules(tagsManager *tags.Manager, finder
 	return vr, nil
 }
 
-func buildValidationResult(rule v1alpha1.TagValidationRule, validationType string) *vapitypes.ValidationRuleResult {
+func buildValidationResult(rule v1alpha1.TagValidationRule) *vapitypes.ValidationRuleResult {
 	state := vapi.ValidationSucceeded
+	validationType := constants.ValidationTypeTag
 
-	validationRule := fmt.Sprintf("%s-%s-%s-%s", vapiconstants.ValidationRulePrefix, "tag", rule.EntityType, rule.Tag)
+	validationRule := fmt.Sprintf("%s-%s-%s-%s", vapiconstants.ValidationRulePrefix, validationType, rule.EntityType, rule.Tag)
 
 	latestCondition := vapi.DefaultValidationCondition()
 	latestCondition.Message = "Required entity tags were found"