From 72e63c7243af00bd8c5f40fc839e0574f48a7549 Mon Sep 17 00:00:00 2001 From: Raffael Date: Tue, 31 Oct 2023 14:59:53 +0100 Subject: [PATCH] 222 add new service attribute spec to present backstage compatible relations (dependsOn, providesApis, consumesApis), tags and labels --- .github/workflows/go.yaml | 2 +- api/generated_apimodel.go | 24 ++++- api/openapi-v3-spec.json | 89 +++++++++++++++++++ go.mod | 2 +- internal/service/services/services.go | 35 ++++++++ internal/service/services/services_test.go | 76 ++++++++++++++++ test/acceptance/servicectl_acc_test.go | 23 +++++ .../service-patch-spec.json | 29 ++++++ 8 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 test/resources/acceptance-expected/service-patch-spec.json diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 757eec1..22d6851 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -15,7 +15,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: '1.20' + go-version: '1.21' - name: Build run: go build diff --git a/api/generated_apimodel.go b/api/generated_apimodel.go index 1aaf7cc..0d08fa3 100644 --- a/api/generated_apimodel.go +++ b/api/generated_apimodel.go @@ -241,7 +241,10 @@ type ServiceCreateDto struct { // The operation type of the service. 'WORKLOAD' follows the default deployment strategy of one instance per environment, 'PLATFORM' one instance per cluster or node and 'APPLICATION' is a standalone application that is not deployed via the common strategies. OperationType *string `yaml:"operationType,omitempty" json:"operationType,omitempty"` // The value defines if the service is available from the internet and the time period in which security holes must be processed. - InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + Tags []string `yaml:"tags,omitempty" json:"tags,omitempty"` + Labels *map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` + Spec *ServiceSpecDto `yaml:"spec,omitempty" json:"spec,omitempty"` // The jira issue to use for committing a change, or the last jira issue used. JiraIssue string `yaml:"-" json:"jiraIssue"` } @@ -262,7 +265,10 @@ type ServiceDto struct { // The operation type of the service. 'WORKLOAD' follows the default deployment strategy of one instance per environment, 'PLATFORM' one instance per cluster or node and 'APPLICATION' is a standalone application that is not deployed via the common strategies. OperationType *string `yaml:"operationType,omitempty" json:"operationType,omitempty"` // The value defines if the service is available from the internet and the time period in which security holes must be processed. - InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + Tags []string `yaml:"tags,omitempty" json:"tags,omitempty"` + Labels *map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` + Spec *ServiceSpecDto `yaml:"spec,omitempty" json:"spec,omitempty"` // ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates. TimeStamp string `yaml:"-" json:"timeStamp"` // The git commit hash this information was originally committed under. When sending an update, include the original commitHash you got so we can detect concurrent updates. @@ -295,7 +301,10 @@ type ServicePatchDto struct { // The operation type of the service. 'WORKLOAD' follows the default deployment strategy of one instance per environment, 'PLATFORM' one instance per cluster or node and 'APPLICATION' is a standalone application that is not deployed via the common strategies. OperationType *string `yaml:"operationType,omitempty" json:"operationType,omitempty"` // The value defines if the service is available from the internet and the time period in which security holes must be processed. - InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + InternetExposed *bool `yaml:"internetExposed,omitempty" json:"internetExposed,omitempty"` + Tags []string `yaml:"tags,omitempty" json:"tags,omitempty"` + Labels *map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` + Spec *ServiceSpecDto `yaml:"spec,omitempty" json:"spec,omitempty"` // ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates. TimeStamp string `yaml:"-" json:"timeStamp"` // The git commit hash this information was originally committed under. When sending an update, include the original commitHash you got so we can detect concurrent updates. @@ -309,3 +318,12 @@ type ServicePatchDto struct { type ServicePromotersDto struct { Promoters []string `yaml:"promoters" json:"promoters"` } + +type ServiceSpecDto struct { + // A relation denoting a dependency on another entity + DependsOn []string `yaml:"dependsOn,omitempty" json:"dependsOn,omitempty"` + // A relation with an API, provided by this entity + ProvidesApis []string `yaml:"providesApis,omitempty" json:"providesApis,omitempty"` + // A relation with an API, consumed by this entity + ConsumesApis []string `yaml:"consumesApis,omitempty" json:"consumesApis,omitempty"` +} diff --git a/api/openapi-v3-spec.json b/api/openapi-v3-spec.json index b1953fb..68ca414 100644 --- a/api/openapi-v3-spec.json +++ b/api/openapi-v3-spec.json @@ -2194,6 +2194,26 @@ "type": "boolean", "description": "The value defines if the service is available from the internet and the time period in which security holes must be processed." }, + "tags": { + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-tag", "other-tag"] + }, + "labels": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "example": { + "some-key": "some-value", + "other-key": "other-value" + } + }, + "spec": { + "$ref": "#/components/schemas/ServiceSpecDto" + }, "timeStamp": { "type": "string", "description": "ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates.", @@ -2286,6 +2306,26 @@ "type": "boolean", "description": "The value defines if the service is available from the internet and the time period in which security holes must be processed." }, + "tags": { + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-tag", "other-tag"] + }, + "labels": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "example": { + "some-key": "some-value", + "other-key": "other-value" + } + }, + "spec": { + "$ref": "#/components/schemas/ServiceSpecDto" + }, "jiraIssue": { "type": "string", "description": "The jira issue to use for committing a change, or the last jira issue used.", @@ -2356,6 +2396,26 @@ "type": "boolean", "description": "The value defines if the service is available from the internet and the time period in which security holes must be processed." }, + "tags": { + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-tag", "other-tag"] + }, + "labels": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "example": { + "some-key": "some-value", + "other-key": "other-value" + } + }, + "spec": { + "$ref": "#/components/schemas/ServiceSpecDto" + }, "timeStamp": { "type": "string", "description": "ISO-8601 UTC date time at which this information was originally committed. When sending an update, include the original timestamp you got so we can detect concurrent updates.", @@ -2431,6 +2491,35 @@ } } }, + "ServiceSpecDto": { + "type": "object", + "properties": { + "dependsOn": { + "description": "A relation denoting a dependency on another entity", + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-service", "other-service"] + }, + "providesApis":{ + "description": "A relation with an API, provided by this entity", + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-api"] + }, + "consumesApis": { + "description": "A relation with an API, consumed by this entity", + "type": "array", + "items": { + "type": "string" + }, + "example": ["some-api", "other-api"] + } + } + }, "Notification": { "type": "object", "description": "Schema of the Dto sent to all configured downstreams upon a change of service.", diff --git a/go.mod b/go.mod index 86e6fc1..15b4d59 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/Interhyp/metadata-service -go 1.20 +go 1.21 // exclude actually unused dependencies (mostly of pact-go, which is testing only anyway) // because our scanner fails to understand they are not in use diff --git a/internal/service/services/services.go b/internal/service/services/services.go index d809dda..bb85e3d 100644 --- a/internal/service/services/services.go +++ b/internal/service/services/services.go @@ -145,6 +145,9 @@ func (s *Impl) mapServiceCreateDtoToServiceDto(serviceCreateDto openapi.ServiceC Description: serviceCreateDto.Description, Lifecycle: &initialServiceLifecycle, InternetExposed: serviceCreateDto.InternetExposed, + Spec: serviceCreateDto.Spec, + Tags: serviceCreateDto.Tags, + Labels: serviceCreateDto.Labels, } } @@ -343,6 +346,23 @@ func patchService(current openapi.ServiceDto, patch openapi.ServicePatchDto) ope Description: patchStringPtr(patch.Description, current.Description), Lifecycle: patchStringPtr(patch.Lifecycle, current.Lifecycle), InternetExposed: patchPtr[bool](patch.InternetExposed, current.InternetExposed), + Spec: patchServiceSpec(patch.Spec, current.Spec), + Tags: patchStringSlice(patch.Tags, current.Tags), + Labels: patchMapSlice(patch.Labels, current.Labels), + } +} + +func patchServiceSpec(patch *openapi.ServiceSpecDto, current *openapi.ServiceSpecDto) *openapi.ServiceSpecDto { + if patch != nil && current != nil { + return &openapi.ServiceSpecDto{ + DependsOn: patchStringSlice(patch.DependsOn, current.DependsOn), + ProvidesApis: patchStringSlice(patch.ProvidesApis, current.ProvidesApis), + ConsumesApis: patchStringSlice(patch.ConsumesApis, current.ConsumesApis), + } + } else if patch != nil { + return patch + } else { + return current } } @@ -364,6 +384,18 @@ func patchStringSlice(patch []string, original []string) []string { } } +func patchMapSlice(patch *map[string]string, original *map[string]string) *map[string]string { + if patch != nil { + if len(*patch) == 0 { + // remove + return nil + } + return patch + } else { + return original + } +} + func patchQuicklinkSlice(patch []openapi.Quicklink, original []openapi.Quicklink) []openapi.Quicklink { if patch != nil { if len(patch) == 0 { @@ -525,6 +557,9 @@ func (s *Impl) validRepoKey(ctx context.Context, candidate string, serviceName s if candidate == serviceName+".helm-deployment" { return nil } + if candidate == serviceName+".none" { + return nil + } return errors.New("repository key must have acceptable name and type combination (allowed types: api implementation helm-deployment), and for helm-deployment the name must match the service name") } diff --git a/internal/service/services/services_test.go b/internal/service/services/services_test.go index 5c6a11d..4bf4f1c 100644 --- a/internal/service/services/services_test.go +++ b/internal/service/services/services_test.go @@ -43,11 +43,41 @@ func tstCurrent() openapi.ServiceDto { } } +func tstCurrentSpec() openapi.ServiceDto { + return openapi.ServiceDto{ + Owner: "owner", + Quicklinks: []openapi.Quicklink{ + { + Url: p("url"), + Title: p("title"), + Description: p("desc"), + }, + }, + Repositories: []string{"repo1", "repo2"}, + AlertTarget: "target", + DevelopmentOnly: b(true), + OperationType: p("PLATFORM"), + TimeStamp: "ts", + CommitHash: "hash", + Lifecycle: p("experimental"), + Spec: &openapi.ServiceSpecDto{ + DependsOn: []string{"other-domain"}, + ProvidesApis: []string{"some-other-api"}, + ConsumesApis: []string{"other-api"}, + }, + } +} + func tstPatchService(t *testing.T, patch openapi.ServicePatchDto, expected openapi.ServiceDto) { actual := patchService(tstCurrent(), patch) require.Equal(t, expected, actual) } +func tstPatchServiceSpec(t *testing.T, patch openapi.ServicePatchDto, expected openapi.ServiceDto) { + actual := patchService(tstCurrentSpec(), patch) + require.Equal(t, expected, actual) +} + func TestPatchService_EmptyPatch(t *testing.T) { docs.Description("patching of services works with an empty patch") expected := tstCurrent() @@ -120,6 +150,52 @@ func TestPatchService_ClearFields(t *testing.T) { }) } +func TestPatchServiceSpec_EmptyPatch(t *testing.T) { + docs.Description("patching of service spec with an empty patch") + expected := tstCurrent() + tstPatchService(t, openapi.ServicePatchDto{ + TimeStamp: "ts", + CommitHash: "hash", + }, expected) +} + +func TestPatchServiceSpec_ReplaceAll(t *testing.T) { + docs.Description("patching of service spec with an empty patch") + expected := tstCurrent() + expected.Spec = &openapi.ServiceSpecDto{ + DependsOn: []string{"some-domain"}, + ProvidesApis: []string{"some-other-api"}, + ConsumesApis: []string{"some-api"}, + } + tstPatchService(t, openapi.ServicePatchDto{ + TimeStamp: "ts", + CommitHash: "hash", + Spec: &openapi.ServiceSpecDto{ + DependsOn: []string{"some-domain"}, + ProvidesApis: []string{"some-other-api"}, + ConsumesApis: []string{"some-api"}, + }, + }, expected) +} + +func TestPatchServiceSpec_ReplaceSpecific(t *testing.T) { + docs.Description("patching of service spec with an empty patch") + expected := tstCurrent() + expected.Spec = &openapi.ServiceSpecDto{ + DependsOn: []string{"some-domain"}, + ConsumesApis: []string{"some-api"}, + } + tstPatchServiceSpec(t, openapi.ServicePatchDto{ + TimeStamp: "ts", + CommitHash: "hash", + Spec: &openapi.ServiceSpecDto{ + DependsOn: []string{"some-domain"}, + ProvidesApis: []string{}, + ConsumesApis: []string{"some-api"}, + }, + }, expected) +} + func tstValid() openapi.ServiceDto { description := "short service description" return openapi.ServiceDto{ diff --git a/test/acceptance/servicectl_acc_test.go b/test/acceptance/servicectl_acc_test.go index f73d7a1..65287ee 100644 --- a/test/acceptance/servicectl_acc_test.go +++ b/test/acceptance/servicectl_acc_test.go @@ -972,6 +972,29 @@ func TestPATCHService_ChangeOwner(t *testing.T) { require.Equal(t, tstServiceMovedExpectedKafka("some-service-backend"), string(actual)) } +func TestPATCHService_ChangeSpec(t *testing.T) { + tstReset() + + docs.Given("Given an authenticated admin user") + token := tstValidAdminToken() + + docs.When("When they perform a valid patch of an existing service that changes its spec") + body := tstServicePatch() + body.Spec = &openapi.ServiceSpecDto{ + DependsOn: []string{"some-service", "other-service"}, + ProvidesApis: []string{}, + ConsumesApis: []string{"some-api"}, + } + response, err := tstPerformPatch("/rest/api/v1/services/some-service-backend", token, &body) + + docs.Then("Then the request is successful and the response is as expected") + tstAssert(t, response, err, http.StatusOK, "service-patch-spec.json") + + docs.Then("And the service has been cached and can be read again, returning the correct spec") + readAgain, err := tstPerformGet("/rest/api/v1/services/some-service-backend", tstUnauthenticated()) + tstAssert(t, readAgain, err, http.StatusOK, "service-patch-spec.json") +} + func TestPATCHService_ImplementationCrossrefAllowed(t *testing.T) { tstReset() diff --git a/test/resources/acceptance-expected/service-patch-spec.json b/test/resources/acceptance-expected/service-patch-spec.json new file mode 100644 index 0000000..dc4e881 --- /dev/null +++ b/test/resources/acceptance-expected/service-patch-spec.json @@ -0,0 +1,29 @@ +{ + "alertTarget": "squad_nothing@some-organisation.com", + "commitHash": "6c8ac2c35791edf9979623c717a2430000000000", + "developmentOnly": false, + "internetExposed": true, + "jiraIssue": "ISSUE-2345", + "lifecycle": "experimental", + "owner": "some-owner", + "quicklinks": [ + { + "title": "Swagger UI", + "url": "/swagger-ui/index.html" + } + ], + "repositories": [ + "some-service-backend.helm-deployment", + "some-service-backend.implementation" + ], + "spec": { + "consumesApis": [ + "some-api" + ], + "dependsOn": [ + "some-service", + "other-service" + ] + }, + "timeStamp": "2022-11-06T18:14:10Z" +} \ No newline at end of file