chore: update get policy flow as additional layer, removing default p…

…olicy
goto · Nov 18, 2024 · c585c1a · c585c1a
1 parent cf662b7
commit c585c1a
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 34 deletions.
diff --git a/api/handler/v1beta1/adapter.go b/api/handler/v1beta1/adapter.go
@@ -201,6 +201,17 @@ func (a *adapter) ToProviderConfigProto(pc *domain.ProviderConfig) (*guardianv1b
 		providerConfigProto.AllowedAccountTypes = pc.AllowedAccountTypes
 	}
 
+	if pc.Policies != nil {
+		policies := []*guardianv1beta1.ProviderPolicy{}
+		for _, p := range pc.Policies {
+			policies = append(policies, &guardianv1beta1.ProviderPolicy{
+				Policy: p.Policy,
+				When:   p.When,
+			})
+		}
+		providerConfigProto.Policies = policies
+	}
+
 	return providerConfigProto, nil
 }
 

diff --git a/api/handler/v1beta1/provider_test.go b/api/handler/v1beta1/provider_test.go
@@ -352,6 +352,16 @@ func (s *GrpcHandlersSuite) TestCreateProvider() {
 						Description: "Please enter your username",
 					},
 				},
+				Policies: []*domain.ProviderPolicy{
+					{
+						When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy1",
+					},
+					{
+						When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy2@3",
+					},
+				},
 			},
 		}
 		expectedResponse := &guardianv1beta1.CreateProviderResponse{
@@ -390,6 +400,16 @@ func (s *GrpcHandlersSuite) TestCreateProvider() {
 							Description: "Please enter your username",
 						},
 					},
+					Policies: []*guardianv1beta1.ProviderPolicy{
+						{
+							When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+							Policy: "policy1",
+						},
+						{
+							When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+							Policy: "policy2@3",
+						},
+					},
 				},
 				CreatedAt: timestamppb.New(timeNow),
 				UpdatedAt: timestamppb.New(timeNow),
@@ -434,6 +454,16 @@ func (s *GrpcHandlersSuite) TestCreateProvider() {
 						Description: "Please enter your username",
 					},
 				},
+				Policies: []*guardianv1beta1.ProviderPolicy{
+					{
+						When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy1",
+					},
+					{
+						When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy2@3",
+					},
+				},
 			},
 		}
 		res, err := s.grpcServer.CreateProvider(context.Background(), req)
@@ -508,6 +538,16 @@ func (s *GrpcHandlersSuite) TestUpdatedProvider() {
 						},
 					},
 				},
+				Policies: []*domain.ProviderPolicy{
+					{
+						When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy1",
+					},
+					{
+						When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy2@3",
+					},
+				},
 			},
 		}
 		expectedResponse := &guardianv1beta1.UpdateProviderResponse{
@@ -534,6 +574,16 @@ func (s *GrpcHandlersSuite) TestUpdatedProvider() {
 							},
 						},
 					},
+					Policies: []*guardianv1beta1.ProviderPolicy{
+						{
+							When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+							Policy: "policy1",
+						},
+						{
+							When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+							Policy: "policy2@3",
+						},
+					},
 				},
 				CreatedAt: timestamppb.New(timeNow),
 				UpdatedAt: timestamppb.New(timeNow),
@@ -566,6 +616,16 @@ func (s *GrpcHandlersSuite) TestUpdatedProvider() {
 						},
 					},
 				},
+				Policies: []*guardianv1beta1.ProviderPolicy{
+					{
+						When:   "$appeal.resource.type == 'project' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy1",
+					},
+					{
+						When:   "$appeal.resource.type == 'dataset' && $appeal.resource.urn == 'data-gojek-id-mart'",
+						Policy: "policy2@3",
+					},
+				},
 			},
 		}
 		res, err := s.grpcServer.UpdateProvider(context.Background(), req)

diff --git a/core/appeal/service.go b/core/appeal/service.go
@@ -1529,52 +1529,54 @@ func (s *Service) checkExtensionEligibility(a *domain.Appeal, p *domain.Provider
 
 func getPolicy(a *domain.Appeal, p *domain.Provider, policiesMap map[string]map[uint]*domain.Policy) (*domain.Policy, error) {
 	var policyConfig domain.PolicyConfig
-	if len(p.Config.Policies) > 0 {
-		appealMap, err := a.ToMap()
-		if err != nil {
-			return nil, fmt.Errorf("parsing appeal struct to map: %w", err)
+	var resourceConfig *domain.ResourceConfig
+	for _, rc := range p.Config.Resources {
+		if rc.Type == a.Resource.Type {
+			resourceConfig = rc
+			break
 		}
+	}
+	if resourceConfig == nil {
+		return nil, fmt.Errorf("%w: couldn't find %q resource type in the provider config", ErrInvalidResourceType, a.Resource.Type)
+	}
+	policyConfig = *resourceConfig.Policy
 
-		providerConfig := p.Config.DefaultPolicy
-		for _, pc := range p.Config.Policies {
-			if pc.When != "" {
-				v, err := evaluator.Expression(pc.When).EvaluateWithVars(map[string]interface{}{
-					"appeal": appealMap,
-				})
-				if err != nil {
-					return nil, err
-				}
+	appealMap, err := a.ToMap()
+	if err != nil {
+		return nil, fmt.Errorf("parsing appeal struct to map: %w", err)
+	}
 
-				isFalsy := reflect.ValueOf(v).IsZero()
-				if isFalsy {
-					continue
-				}
+	var dynamicPolicyConfigData string
+	for _, pc := range p.Config.Policies {
+		if pc.When != "" {
+			v, err := evaluator.Expression(pc.When).EvaluateWithVars(map[string]interface{}{
+				"appeal": appealMap,
+			})
+			if err != nil {
+				return nil, err
+			}
 
-				providerConfig = pc.Policy
+			isFalsy := reflect.ValueOf(v).IsZero()
+			if isFalsy {
+				continue
 			}
+
+			dynamicPolicyConfigData = pc.Policy
 		}
+	}
 
-		policyData := strings.Split(providerConfig, "@")
-		policyConfig.ID = policyData[0]
+	if dynamicPolicyConfigData != "" {
+		var dynamicPolicyConfig domain.PolicyConfig
+		policyData := strings.Split(dynamicPolicyConfigData, "@")
+		dynamicPolicyConfig.ID = policyData[0]
 		if len(policyData) > 1 {
 			version, err := strconv.Atoi(policyData[1])
 			if err != nil {
 				return nil, fmt.Errorf("invalid policy version: %w", err)
 			}
-			policyConfig.Version = version
-		}
-	} else {
-		var resourceConfig *domain.ResourceConfig
-		for _, rc := range p.Config.Resources {
-			if rc.Type == a.Resource.Type {
-				resourceConfig = rc
-				break
-			}
-		}
-		if resourceConfig == nil {
-			return nil, fmt.Errorf("%w: couldn't find %q resource type in the provider config", ErrInvalidResourceType, a.Resource.Type)
+			dynamicPolicyConfig.Version = version
 		}
-		policyConfig = *resourceConfig.Policy
+		policyConfig = dynamicPolicyConfig
 	}
 
 	policy, ok := policiesMap[policyConfig.ID][uint(policyConfig.Version)]

diff --git a/core/appeal/service_test.go b/core/appeal/service_test.go
@@ -1645,7 +1645,6 @@ func (s *ServiceTestSuite) TestCreate() {
 							Policy: "policy_dataset",
 						},
 					},
-					DefaultPolicy: "policy_table",
 				},
 			},
 		}
-Original file line number
+Diff line change
@@ Expand Up / @@ -1645,7 +1645,6 @@ func (s *ServiceTestSuite) TestCreate() { @@
     							Policy: "policy_dataset",
     						},
     					},
-    					DefaultPolicy: "policy_table",
     				},
     			},
     		}
@@ Expand Down @@