Repository: clouditor/clouditor
Pull request #681 (2022-05-13 06:56:23)

Implement hooks for updated cloud services
This PR adds hooks for updates made to Cloud Service objects, so other components can be informed, e.g. when a cloud service's requirements change

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#681](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (c86f5a4) into [main](https://codecov.io/gh/clouditor/clouditor/commit/1a6be5b4be15b98ca90ec368a83e5cab0a76ecaa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (1a6be5b) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
+ Coverage   81.41%   81.46%   +0.05%     
==========================================
  Files          65       65              
  Lines        5929     5945      +16     
==========================================
+ Hits         4827     4843      +16     
  Misses        850      850              
  Partials      252      252              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `81.46% <100.00%> (+0.05%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [api/orchestrator/orchestrator.go](https://codecov.io/gh/clouditor/clouditor/pull/681/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-YXBpL29yY2hlc3RyYXRvci9vcmNoZXN0cmF0b3IuZ28=) | `100.00% <ø> (ø)` | |
| [service/orchestrator/cloud\_service.go](https://codecov.io/gh/clouditor/clouditor/pull/681/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9vcmNoZXN0cmF0b3IvY2xvdWRfc2VydmljZS5nbw==) | `71.69% <100.00%> (+0.26%)` | :arrow_up: |
| [service/orchestrator/orchestrator.go](https://codecov.io/gh/clouditor/clouditor/pull/681/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9vcmNoZXN0cmF0b3Ivb3JjaGVzdHJhdG9yLmdv) | `74.43% <100.00%> (+1.84%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [1a6be5b...c86f5a4](https://codecov.io/gh/clouditor/clouditor/pull/681?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).


Patch:
diff --git a/api/orchestrator/orchestrator.go b/api/orchestrator/orchestrator.go
index 51d7c78e..80870e90 100644
--- a/api/orchestrator/orchestrator.go
+++ b/api/orchestrator/orchestrator.go
@@ -31,6 +31,8 @@ import (
 	"strings"
 )
 
+type CloudServiceHookFunc func(cld *CloudService, err error)
+
 var (
 	ErrRequestIsNil     = errors.New("request is empty")
 	ErrCertificateIsNil = errors.New("certificate is empty")
diff --git a/service/orchestrator/cloud_service.go b/service/orchestrator/cloud_service.go
index d12fd13c..17640fd3 100644
--- a/service/orchestrator/cloud_service.go
+++ b/service/orchestrator/cloud_service.go
@@ -134,6 +134,7 @@ func (s *Service) UpdateCloudService(_ context.Context, req *orchestrator.Update
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "database error: %v", err)
 	}
+	go s.informHooks(response, nil)
 	return
 }
 
diff --git a/service/orchestrator/orchestrator.go b/service/orchestrator/orchestrator.go
index c2e62c71..4b14c9a9 100644
--- a/service/orchestrator/orchestrator.go
+++ b/service/orchestrator/orchestrator.go
@@ -67,6 +67,12 @@ type Service struct {
 	// mm is a mutex for metric related maps
 	mm sync.Mutex
 
+	// cloudServiceHooks is a list of hook functions that can be used to inform
+	// about updated CloudServices
+	cloudServiceHooks []orchestrator.CloudServiceHookFunc
+	// hookMutex is used for (un)locking hook calls
+	hookMutex sync.RWMutex
+
 	// Currently only in-memory
 	results map[string]*assessment.AssessmentResult
 
@@ -158,6 +164,27 @@ func NewService(opts ...ServiceOption) *Service {
 	return &s
 }
 
+// informHooks informs the registered hook functions
+func (s *Service) informHooks(cld *orchestrator.CloudService, err error) {
+	s.hookMutex.RLock()
+	hooks := s.cloudServiceHooks
+	defer s.hookMutex.RUnlock()
+
+	// Inform our hook, if we have any
+	if len(hooks) > 0 {
+		for _, hook := range hooks {
+			// We could do hook concurrent again (assuming different hooks don't interfere with each other)
+			hook(cld, err)
+		}
+	}
+}
+
+func (s *Service) RegisterCloudServiceHook(hook orchestrator.CloudServiceHookFunc) {
+	s.hookMutex.Lock()
+	defer s.hookMutex.Unlock()
+	s.cloudServiceHooks = append(s.cloudServiceHooks, hook)
+}
+
 // StoreAssessmentResult is a method implementation of the orchestrator interface: It receives an assessment result and stores it
 func (s *Service) StoreAssessmentResult(_ context.Context, req *orchestrator.StoreAssessmentResultRequest) (resp *orchestrator.StoreAssessmentResultResponse, err error) {
 	_, err = req.Result.Validate()


Test patch:
diff --git a/service/orchestrator/orchestrator_test.go b/service/orchestrator/orchestrator_test.go
index 97a0cb02..b04c6c04 100644
--- a/service/orchestrator/orchestrator_test.go
+++ b/service/orchestrator/orchestrator_test.go
@@ -844,3 +844,103 @@ func Test_ListCertificates(t *testing.T) {
 	assert.NotEmpty(t, listCertificatesResponse.Certificates)
 	assert.Equal(t, len(listCertificatesResponse.Certificates), 1)
 }
+
+func TestCloudServiceHooks(t *testing.T) {
+	var (
+		hookCallCounter = 0
+		wg              sync.WaitGroup
+		hookCounts      = 2
+	)
+
+	wg.Add(hookCounts)
+
+	firstHookFunction := func(cloudService *orchestrator.CloudService, err error) {
+		hookCallCounter++
+		log.Println("Hello from inside the firstHookFunction")
+		wg.Done()
+	}
+
+	secondHookFunction := func(cloudService *orchestrator.CloudService, err error) {
+		hookCallCounter++
+		log.Println("Hello from inside the secondHookFunction")
+		wg.Done()
+	}
+
+	type args struct {
+		in0               context.Context
+		serviceUpdate     *orchestrator.UpdateCloudServiceRequest
+		cloudServiceHooks []orchestrator.CloudServiceHookFunc
+	}
+	tests := []struct {
+		name     string
+		args     args
+		wantResp *orchestrator.CloudService
+		wantErr  bool
+	}{
+		{
+			name: "Update Cloud Service",
+			args: args{
+				in0: context.TODO(),
+				serviceUpdate: &orchestrator.UpdateCloudServiceRequest{
+					ServiceId: "00000000-0000-0000-000000000000",
+					Service: &orchestrator.CloudService{
+						Id:          "00000000-0000-0000-000000000000",
+						Name:        "test service",
+						Description: "test service",
+						Requirements: &orchestrator.CloudService_Requirements{
+							RequirementIds: []string{"1", "2", "3"},
+						},
+					},
+				},
+				cloudServiceHooks: []orchestrator.CloudServiceHookFunc{firstHookFunction, secondHookFunction},
+			},
+			wantErr: false,
+			wantResp: &orchestrator.CloudService{
+				Id:          "00000000-0000-0000-000000000000",
+				Name:        "test service",
+				Description: "test service",
+				Requirements: &orchestrator.CloudService_Requirements{
+					RequirementIds: []string{"1", "2", "3"},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hookCallCounter = 0
+			s := NewService()
+
+			_, err := s.CreateDefaultTargetCloudService()
+			if err != nil {
+				t.Errorf("CreateCloudService() error = %v", err)
+			}
+
+			for i, hookFunction := range tt.args.cloudServiceHooks {
+				s.RegisterCloudServiceHook(hookFunction)
+
+				// Check if hook is registered
+				funcName1 := runtime.FuncForPC(reflect.ValueOf(s.cloudServiceHooks[i]).Pointer()).Name()
+				funcName2 := runtime.FuncForPC(reflect.ValueOf(hookFunction).Pointer()).Name()
+				assert.Equal(t, funcName1, funcName2)
+			}
+
+			// To test the hooks we have to call a function that calls the hook function
+			gotResp, err := s.UpdateCloudService(tt.args.in0, tt.args.serviceUpdate)
+
+			// wait for all hooks (2 services * 2 hooks)
+			wg.Wait()
+
+			if (err != nil) != tt.wantErr {
+				t.Errorf("UpdateCloudService() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(gotResp, tt.wantResp) {
+				t.Errorf("UpdateCloudService() gotResp = %v, want %v", gotResp, tt.wantResp)
+			}
+
+			assert.Equal(t, tt.wantResp, gotResp)
+			assert.Equal(t, hookCounts, hookCallCounter)
+		})
+	}
+}