Merge pull request #473 from validator-labs/refactor/validation-rule-interface

refactor: make each rule implement `validationrule.Interface`

Author: Matt Welke

api/v1alpha1/awsvalidator_types.go

@@ -23,6 +23,8 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/validator-labs/validator/pkg/validationrule"
+
 	"github.com/validator-labs/validator-plugin-aws/pkg/constants"
 )
 
@@ -97,11 +99,25 @@ type AwsSTSAuth struct {
 // Each AmiRule is intended to match a single AMI, as an AmiRule is considered successful if at least one AMI is found.
 // Refer to https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeImages.html for more information.
 type AmiRule struct {
-	Name    string   `json:"name" yaml:"name"`
-	AmiIDs  []string `json:"amiIds,omitempty" yaml:"amiIds,omitempty"`
-	Filters []Filter `json:"filters,omitempty" yaml:"filters,omitempty"`
-	Owners  []string `json:"owners,omitempty" yaml:"owners,omitempty"`
-	Region  string   `json:"region" yaml:"region"`
+	validationrule.ManuallyNamed `json:"-"`
+
+	RuleName string   `json:"name" yaml:"name"`
+	AmiIDs   []string `json:"amiIds,omitempty" yaml:"amiIds,omitempty"`
+	Filters  []Filter `json:"filters,omitempty" yaml:"filters,omitempty"`
+	Owners   []string `json:"owners,omitempty" yaml:"owners,omitempty"`
+	Region   string   `json:"region" yaml:"region"`
+}
+
+var _ validationrule.Interface = (*AmiRule)(nil)
+
+// Name returns the name of the AmiRule.
+func (r AmiRule) Name() string {
+	return r.RuleName
+}
+
+// SetName sets the name of the AmiRule.
+func (r *AmiRule) SetName(name string) {
+	r.RuleName = name
 }
 
 // Filter defines a filter to apply to an AWS API query.
@@ -113,10 +129,14 @@ type Filter struct {
 
 // IamRoleRule compares the IAM permissions associated with an IAM role against an expected permission set.
 type IamRoleRule struct {
+	validationrule.AutomaticallyNamed `json:"-"`
+
 	IamRoleName string           `json:"iamRoleName" yaml:"iamRoleName"`
 	Policies    []PolicyDocument `json:"iamPolicies" yaml:"iamPolicies"`
 }
 
+var _ validationrule.Interface = (*IamRoleRule)(nil)
+
 // Name returns the name of an IamRoleRule.
 func (r IamRoleRule) Name() string {
 	return r.IamRoleName
@@ -129,10 +149,14 @@ func (r IamRoleRule) IAMPolicies() []PolicyDocument {
 
 // IamUserRule compares the IAM permissions associated with an IAM user against an expected permission set.
 type IamUserRule struct {
+	validationrule.AutomaticallyNamed `json:"-"`
+
 	IamUserName string           `json:"iamUserName" yaml:"iamUserName"`
 	Policies    []PolicyDocument `json:"iamPolicies" yaml:"iamPolicies"`
 }
 
+var _ validationrule.Interface = (*IamUserRule)(nil)
+
 // Name returns the name of an IamUserRule.
 func (r IamUserRule) Name() string {
 	return r.IamUserName
@@ -145,10 +169,14 @@ func (r IamUserRule) IAMPolicies() []PolicyDocument {
 
 // IamGroupRule compares the IAM permissions associated with an IAM group against an expected permission set.
 type IamGroupRule struct {
+	validationrule.AutomaticallyNamed `json:"-"`
+
 	IamGroupName string           `json:"iamGroupName" yaml:"iamGroupName"`
 	Policies     []PolicyDocument `json:"iamPolicies" yaml:"iamPolicies"`
 }
 
+var _ validationrule.Interface = (*IamGroupRule)(nil)
+
 // Name returns the name of an IamGroupRule.
 func (r IamGroupRule) Name() string {
 	return r.IamGroupName
@@ -161,10 +189,14 @@ func (r IamGroupRule) IAMPolicies() []PolicyDocument {
 
 // IamPolicyRule compares the IAM permissions associated with an IAM policy against an expected permission set.
 type IamPolicyRule struct {
+	validationrule.AutomaticallyNamed `json:"-"`
+
 	IamPolicyARN string           `json:"iamPolicyArn" yaml:"iamPolicyArn"`
 	Policies     []PolicyDocument `json:"iamPolicies" yaml:"iamPolicies"`
 }
 
+var _ validationrule.Interface = (*IamPolicyRule)(nil)
+
 // Name returns the name of an IamPolicyRule.
 func (r IamPolicyRule) Name() string {
 	return r.IamPolicyARN
@@ -208,12 +240,26 @@ func (c Condition) String() string {
 
 // ServiceQuotaRule ensures that AWS service quotas are within a particular threshold.
 type ServiceQuotaRule struct {
-	Name          string         `json:"name" yaml:"name"`
+	validationrule.ManuallyNamed `json:"-"`
+
+	RuleName      string         `json:"name" yaml:"name"`
 	Region        string         `json:"region" yaml:"region"`
 	ServiceCode   string         `json:"serviceCode" yaml:"serviceCode"`
 	ServiceQuotas []ServiceQuota `json:"serviceQuotas" yaml:"serviceQuotas"`
 }
 
+var _ validationrule.Interface = (*ServiceQuotaRule)(nil)
+
+// Name returns the name of the ServiceQuotaRule.
+func (r ServiceQuotaRule) Name() string {
+	return r.RuleName
+}
+
+// SetName sets the name of the ServiceQuotaRule.
+func (r *ServiceQuotaRule) SetName(name string) {
+	r.RuleName = name
+}
+
 // ServiceQuota defines an AWS service quota and an associated buffer.
 type ServiceQuota struct {
 	Name   string `json:"name" yaml:"name"`
@@ -222,14 +268,28 @@ type ServiceQuota struct {
 
 // TagRule ensures that the tags associated with a particular AWS resource match an expected tag set.
 type TagRule struct {
-	Name          string   `json:"name" yaml:"name"`
+	validationrule.ManuallyNamed `json:"-"`
+
+	RuleName      string   `json:"name" yaml:"name"`
 	Key           string   `json:"key" yaml:"key"`
 	ExpectedValue string   `json:"expectedValue" yaml:"expectedValue"`
 	Region        string   `json:"region" yaml:"region"`
 	ResourceType  string   `json:"resourceType" yaml:"resourceType"`
 	ARNs          []string `json:"arns" yaml:"arns"`
 }
 
+var _ validationrule.Interface = (*TagRule)(nil)
+
+// Name returns the name of the ServiceQuotaRule.
+func (r TagRule) Name() string {
+	return r.RuleName
+}
+
+// SetName sets the name of the ServiceQuotaRule.
+func (r *TagRule) SetName(name string) {
+	r.RuleName = name
+}
+
 // AwsValidatorStatus defines the observed state of AwsValidator
 type AwsValidatorStatus struct{}
 

api/v1alpha1/zz_generated.deepcopy.go

@@ -18,7 +18,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.20.1
 	github.com/onsi/gomega v1.34.1
 	github.com/pkg/errors v0.9.1
-	github.com/validator-labs/validator v0.1.6
+	github.com/validator-labs/validator v0.1.7
 	golang.org/x/exp v0.0.0-20240808152545-0cdaa3abc0fa
 	k8s.io/api v0.31.0
 	k8s.io/apimachinery v0.31.0

go.mod

@@ -206,8 +206,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-github.com/validator-labs/validator v0.1.6 h1:Gckp+rYFPNYJwCU5iLgJXuS/o6yy7ayivvGzhxNOf3o=
-github.com/validator-labs/validator v0.1.6/go.mod h1:ssEvc9ws3kwWJ2VpIsOtgm7WmA6bdux2kkuIHbgT6oU=
+github.com/validator-labs/validator v0.1.7 h1:x1iBKoecChM52G7bbacMsdIQYvB84C65DRIex8TG6vQ=
+github.com/validator-labs/validator v0.1.7/go.mod h1:ssEvc9ws3kwWJ2VpIsOtgm7WmA6bdux2kkuIHbgT6oU=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
 github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

go.sum

@@ -50,9 +50,9 @@ var _ = Describe("AWSValidator controller", Ordered, func() {
 			DefaultRegion: "us-west-1",
 			AmiRules: []v1alpha1.AmiRule{
 				{
-					Name:   "AMIRule",
-					AmiIDs: []string{"ami-12345678"},
-					Region: "us-west-2",
+					RuleName: "AMIRule",
+					AmiIDs:   []string{"ami-12345678"},
+					Region:   "us-west-2",
 				},
 			},
 			IamRoleRules: []v1alpha1.IamRoleRule{
@@ -81,7 +81,7 @@ var _ = Describe("AWSValidator controller", Ordered, func() {
 			},
 			ServiceQuotaRules: []v1alpha1.ServiceQuotaRule{
 				{
-					Name:        "ServiceQuotaRule",
+					RuleName:    "ServiceQuotaRule",
 					Region:      "us-west-1",
 					ServiceCode: "ec2",
 					ServiceQuotas: []v1alpha1.ServiceQuota{

internal/controller/awsvalidator_controller_test.go

@@ -29,7 +29,7 @@ func Validate(spec v1alpha1.AwsValidatorSpec, log logr.Logger) types.ValidationR
 		if err != nil {
 			errMsg := "Failed to reconcile AMI rule"
 			log.V(0).Error(err, errMsg)
-			vrr := validators.BuildValidationResult(rule.Name, errMsg, constants.ValidationTypeAmi)
+			vrr := validators.BuildValidationResult(rule.Name(), errMsg, constants.ValidationTypeAmi)
 			resp.AddResult(vrr, err)
 			continue
 		}
@@ -84,7 +84,7 @@ func Validate(spec v1alpha1.AwsValidatorSpec, log logr.Logger) types.ValidationR
 		if err != nil {
 			errMsg := "Failed to reconcile Service Quota rule"
 			log.V(0).Error(err, errMsg)
-			vrr := validators.BuildValidationResult(rule.Name, errMsg, constants.ValidationTypeServiceQuota)
+			vrr := validators.BuildValidationResult(rule.Name(), errMsg, constants.ValidationTypeServiceQuota)
 			resp.AddResult(vrr, err)
 			continue
 		}
@@ -109,7 +109,7 @@ func Validate(spec v1alpha1.AwsValidatorSpec, log logr.Logger) types.ValidationR
 		if err != nil {
 			errMsg := "Failed to reconcile Tag rule"
 			log.V(0).Error(err, errMsg)
-			vrr := validators.BuildValidationResult(rule.Name, errMsg, constants.ValidationTypeTag)
+			vrr := validators.BuildValidationResult(rule.Name(), errMsg, constants.ValidationTypeTag)
 			resp.AddResult(vrr, err)
 			continue
 		}

pkg/validate/validate.go

@@ -42,7 +42,7 @@ func NewAmiRuleService(log logr.Logger, amiSvc amiAPI) *RuleService {
 func (s *RuleService) ReconcileAmiRule(rule v1alpha1.AmiRule) (*vapitypes.ValidationRuleResult, error) {
 
 	// Build the default latest condition for this AMI rule
-	vr := validators.BuildValidationResult(rule.Name, "All required AMIs were found", constants.ValidationTypeAmi)
+	vr := validators.BuildValidationResult(rule.Name(), "All required AMIs were found", constants.ValidationTypeAmi)
 
 	// Describe AMIs matching the rule. There should be at least one.
 	input := &ec2.DescribeImagesInput{

pkg/validators/ami/ami_validator.go

@@ -51,9 +51,9 @@ func TestAmiValidation(t *testing.T) {
 		{
 			name: "Fail (missing image)",
 			rule: v1alpha1.AmiRule{
-				Name:   "AMI Rule Fail",
-				AmiIDs: []string{"ami-87654321"},
-				Region: "us-west-1",
+				RuleName: "AMI Rule Fail",
+				AmiIDs:   []string{"ami-87654321"},
+				Region:   "us-west-1",
 			},
 			expectedResult: types.ValidationRuleResult{
 				Condition: &vapi.ValidationCondition{
@@ -70,9 +70,9 @@ func TestAmiValidation(t *testing.T) {
 		{
 			name: "Pass",
 			rule: v1alpha1.AmiRule{
-				Name:   "AMI Rule Pass",
-				AmiIDs: []string{"ami-12345678"},
-				Region: "us-west-1",
+				RuleName: "AMI Rule Pass",
+				AmiIDs:   []string{"ami-12345678"},
+				Region:   "us-west-1",
 			},
 			expectedResult: types.ValidationRuleResult{
 				Condition: &vapi.ValidationCondition{

pkg/validators/ami/ami_validator_test.go

@@ -110,7 +110,7 @@ func (s *RuleService) ReconcileServiceQuotaRule(rule v1alpha1.ServiceQuotaRule)
 
 	// Build the default latest condition for this service quota rule
 	vr := validators.BuildValidationResult(
-		rule.Name, "Usage for all service quotas is below specified buffer", constants.ValidationTypeServiceQuota,
+		rule.Name(), "Usage for all service quotas is below specified buffer", constants.ValidationTypeServiceQuota,
 	)
 
 	// Fetch the quota by service code & name & compare against usage