fix: logs

goto · Nov 13, 2024 · 04e1744 · 04e1744
1 parent ade1a41
commit 04e1744
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 37 deletions.
diff --git a/core/provider/common.go b/core/provider/common.go
@@ -57,19 +57,12 @@ func normalizeDetails(details map[string]interface{}) (map[string]interface{}, e
 func compareResources(existingResource, newResource domain.Resource) (bool, string) {
 	opts := cmp.Options{
 		cmpopts.IgnoreFields(domain.Resource{}, "ID", "CreatedAt", "UpdatedAt", "ParentID", "Children"),
-		cmpopts.SortSlices(func(x, y map[string]any) bool {
-			return x["name"].(string) < y["name"].(string) // Assumes each entry has a unique name field
-		}),
 		cmpopts.EquateEmpty(),
 	}
-	normalizedExistingDetails, _ := normalizeDetails(existingResource.Details)
-	normalizedNewDetails, _ := normalizeDetails(newResource.Details)
-	existingResource.Details = normalizedExistingDetails
-	newResource.Details = normalizedNewDetails
-	if diff := cmp.Diff(existingResource, newResource, opts); diff != "" {
-		return true, diff
-	}
-	return false, ""
+	existingResource.Details, _ = normalizeDetails(existingResource.Details)
+	newResource.Details, _ = normalizeDetails(newResource.Details)
+	diff := cmp.Diff(existingResource, newResource, opts)
+	return diff != "", diff
 }
 
 type UnimplementedClient struct{}

diff --git a/core/provider/service.go b/core/provider/service.go
@@ -152,7 +152,7 @@ func (s *Service) Create(ctx context.Context, p *domain.Provider) error {
 	go func() {
 		s.logger.Info(ctx, "provider create fetching resources", "provider_urn", p.URN)
 		ctx := audit.WithActor(context.Background(), domain.SystemActorName)
-		resources, err := s.getResources(ctx, p, nil, nil)
+		_, resources, err := s.fetchNewResources(ctx, p)
 		if err != nil {
 			s.logger.Error(ctx, "failed to fetch resources", "error", err)
 		}
@@ -236,7 +236,7 @@ func (s *Service) Update(ctx context.Context, p *domain.Provider) error {
 	go func() {
 		s.logger.Info(ctx, "provider update fetching resources", "provider_urn", p.URN)
 		ctx := audit.WithActor(context.Background(), domain.SystemActorName)
-		resources, err := s.getResources(ctx, p, nil, nil)
+		_, resources, err := s.fetchNewResources(ctx, p)
 		if err != nil {
 			s.logger.Error(ctx, "failed to fetch resources", "error", err)
 		}
@@ -266,16 +266,17 @@ func (s *Service) FetchResources(ctx context.Context) error {
 	for _, p := range providers {
 		startTime := time.Now()
 		s.logger.Info(ctx, "fetching resources", "provider_urn", p.URN)
-		resources, err := s.getResources(ctx, p, &resourcesCount, &upsertedResourceCount)
+		newResourcesCount, resources, err := s.fetchNewResources(ctx, p)
 		if err != nil {
 			s.logger.Error(ctx, "failed to get resources", "error", err)
 			continue
 		}
+		resourcesCount += newResourcesCount
+		upsertedResourceCount += len(resources)
 		if len(resources) == 0 {
 			s.logger.Info(ctx, "no changes in this provider", "provider_urn", p.URN)
 			continue
 		}
-
 		s.logger.Info(ctx, "resources added", "provider_urn", p.URN, "count", len(flattenResources(resources)))
 		if err := s.resourceService.BulkUpsert(ctx, resources); err != nil {
 			failedProviders[p.URN] = err
@@ -568,18 +569,18 @@ func (s *Service) IsExclusiveRoleAssignment(ctx context.Context, providerType, r
 	return false
 }
 
-func (s *Service) getResources(ctx context.Context, p *domain.Provider, resourcesCount, upsertedResourcesCount *int) ([]*domain.Resource, error) {
+func (s *Service) fetchNewResources(ctx context.Context, p *domain.Provider) (int, []*domain.Resource, error) {
 	c := s.getClient(p.Type)
 	if c == nil {
-		return nil, fmt.Errorf("%w: %v", ErrInvalidProviderType, p.Type)
+		return 0, nil, fmt.Errorf("%w: %v", ErrInvalidProviderType, p.Type)
 	}
 
 	existingGuardianResources, err := s.resourceService.Find(ctx, domain.ListResourcesFilter{
 		ProviderType: p.Type,
 		ProviderURN:  p.URN,
 	})
 	if err != nil {
-		return nil, err
+		return 0, nil, err
 	}
 
 	resourceTypeFilterMap := make(map[string]string)
@@ -591,14 +592,14 @@ func (s *Service) getResources(ctx context.Context, p *domain.Provider, resource
 
 	newProviderResources, err := c.GetResources(ctx, p.Config)
 	if err != nil {
-		return nil, fmt.Errorf("error fetching resources for %v: %w", p.ID, err)
+		return 0, nil, fmt.Errorf("error fetching resources for %v: %w", p.ID, err)
 	}
 	filteredResources := make([]*domain.Resource, 0)
 	for _, r := range newProviderResources {
 		if filterExpression, ok := resourceTypeFilterMap[r.Type]; ok {
 			v, err := evaluator.Expression(filterExpression).EvaluateWithStruct(r)
 			if err != nil {
-				return nil, err
+				return 0, nil, err
 			}
 			if !reflect.ValueOf(v).IsZero() {
 				filteredResources = append(filteredResources, r)
@@ -610,13 +611,12 @@ func (s *Service) getResources(ctx context.Context, p *domain.Provider, resource
 	flattenedProviderResources := flattenResources(filteredResources)
 
 	existingProviderResources := map[string]bool{}
-	if resourcesCount != nil {
-		*resourcesCount += len(flattenedProviderResources)
-	}
-	UpdatedResourceCount := 0
+	updatedResources := []*domain.Resource{}
 	for _, newResource := range flattenedProviderResources {
+		found := false
 		for _, existingResource := range existingGuardianResources {
 			if existingResource.Type == newResource.Type && existingResource.URN == newResource.URN {
+				found = true
 				if existingDetails := existingResource.Details; existingDetails != nil {
 					if newResource.Details != nil {
 						for key, value := range existingDetails {
@@ -628,34 +628,31 @@ func (s *Service) getResources(ctx context.Context, p *domain.Provider, resource
 						newResource.Details = existingDetails
 					}
 					if isUpdated, diff := compareResources(*existingResource, *newResource); isUpdated {
-						s.logger.Info(ctx, "diff", "resources", diff)
-						UpdatedResourceCount++
-						if upsertedResourcesCount != nil {
-							*upsertedResourcesCount++
-						}
-						s.logger.Info(ctx, "Resources is updated", "resource", newResource.Name)
+						s.logger.Debug(ctx, "diff", "resources", diff)
+						updatedResources = append(updatedResources, newResource)
+						s.logger.Info(ctx, "resources is updated", "resource", newResource.Name)
 					}
 				}
 				existingProviderResources[existingResource.ID] = true
 				break
 			}
 		}
-	}
-	if UpdatedResourceCount == 0 && len(existingGuardianResources) == len(flattenedProviderResources) {
-		return []*domain.Resource{}, nil
+		if !found {
+			updatedResources = append(updatedResources, newResource)
+			s.logger.Info(ctx, "new resource added", "resource", newResource.Name)
+		}
 	}
 
 	// mark IsDeleted of guardian resources that no longer exist in provider
-	updatedResources := []*domain.Resource{}
 	for _, r := range existingGuardianResources {
 		if _, ok := existingProviderResources[r.ID]; !ok {
 			r.IsDeleted = true
 			updatedResources = append(updatedResources, r)
+			s.logger.Info(ctx, "resource deleted", "resource", r.Name)
 		}
 	}
 
-	newProviderResources = append(filteredResources, updatedResources...)
-	return newProviderResources, nil
+	return len(newProviderResources), updatedResources, nil
 }
 
 func (s *Service) validateAppealParam(a *domain.Appeal) error {

diff --git a/core/provider/service_test.go b/core/provider/service_test.go
@@ -357,6 +357,22 @@ func (s *ServiceTestSuite) TestFetchResources() {
 			Config: &domain.ProviderConfig{},
 		},
 	}
+
+	s.Run("should return error if got any from resource service", func() {
+		s.mockProviderRepository.EXPECT().Find(mock.MatchedBy(func(ctx context.Context) bool { return true })).Return(providers, nil).Once()
+		for _, p := range providers {
+			s.mockProvider.On("GetResources", mockCtx, p.Config).Return([]*domain.Resource{
+				{ID: "test"},
+			}, nil).Once()
+		}
+		expectedError := errors.New("failed to add resources for providers: [mock_provider]")
+		s.mockResourceService.On("BulkUpsert", mock.Anything, mock.Anything).Return(expectedError).Once()
+		s.mockResourceService.On("Find", mock.Anything, mock.Anything).Return([]*domain.Resource{}, nil).Once()
+		actualError := s.service.FetchResources(context.Background())
+
+		s.EqualError(actualError, expectedError.Error())
+	})
+
 	s.Run("should not upsert any resources when there is no changes", func() {
 		existingResources := []*domain.Resource{
 			{

diff --git a/plugins/providers/grafana/client_test.go b/plugins/providers/grafana/client_test.go
@@ -90,7 +90,6 @@ func TestClient(t *testing.T) {
 	suite.Run(t, new(ClientTestSuite))
 }
 func (s *ClientTestSuite) setup() {
-
 	s.mockHttpClient = new(mocks.HTTPClient)
 
 	s.host = "http://localhost"