diff --git a/pkg/apic/apiservice_test.go b/pkg/apic/apiservice_test.go index 8bdc0c76f..9bb2c3b6b 100644 --- a/pkg/apic/apiservice_test.go +++ b/pkg/apic/apiservice_test.go @@ -350,29 +350,128 @@ func Test_PublishServiceError(t *testing.T) { func Test_processRevision(t *testing.T) { client, httpClient := GetTestServiceClient() - // tests for updating existing revision - httpClient.SetResponses([]api.MockResponse{ - { - FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision - RespCode: http.StatusOK, + testCases := map[string]struct { + skip bool + httpResponses []api.MockResponse + serviceBody ServiceBody + expectedRevName string + }{ + "publish new revision": { + httpResponses: []api.MockResponse{ + { + FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision + RespCode: http.StatusOK, + }, + { + FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision x-agent-details + RespCode: http.StatusOK, + }, + { + FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision + RespCode: http.StatusOK, + }, + { + FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision x-agent-details + RespCode: http.StatusOK, + }, + }, + serviceBody: ServiceBody{ + APIName: "daleapi", + Documentation: []byte("\"docs\""), + Image: "abcde", + ImageContentType: "image/jpeg", + ResourceType: Oas2, + RestAPIID: "12345", + }, + expectedRevName: "daleapi", }, - { - FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision x-agent-details - RespCode: http.StatusOK, + "skip publish when no changes": { + httpResponses: []api.MockResponse{ + { + RespData: `[{"name": "daleapi","tags": ["tag1","tag2"]}]`, + RespCode: http.StatusOK, + }, + }, + serviceBody: ServiceBody{ + APIName: "daleapi", + Documentation: []byte("\"docs\""), + Image: "abcde", + ImageContentType: "image/jpeg", + ResourceType: Oas2, + RestAPIID: "12345", + specHash: "abc123", + specHashes: map[string]interface{}{ + "abc123": "daleapi", + }, + serviceContext: serviceContext{ + serviceAction: updateAPI, + }, + }, + expectedRevName: "daleapi", }, - { - FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision - RespCode: http.StatusOK, + "skip publish when previous revision found": { + httpResponses: []api.MockResponse{ + { + RespData: `[{"name": "daleapi","tags": ["tag1","tag2"]},{"name": "daleapi-1","tags": ["tag1","tag2"]}]`, + RespCode: http.StatusOK, + }, + }, + serviceBody: ServiceBody{ + APIName: "daleapi", + Documentation: []byte("\"docs\""), + Image: "abcde", + ImageContentType: "image/jpeg", + ResourceType: Oas2, + RestAPIID: "12345", + specHash: "abc123", + specHashes: map[string]interface{}{ + "abc123": "daleapi-1", + }, + serviceContext: serviceContext{ + serviceAction: updateAPI, + }, + }, + expectedRevName: "daleapi-1", }, - { - FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision x-agent-details - RespCode: http.StatusOK, + "find revision match using original spec hash": { + httpResponses: []api.MockResponse{ + { + RespData: `[{"name": "daleapi","tags": ["tag1","tag2"]},{"name": "daleapi-1","tags": ["tag1","tag2"]}]`, + RespCode: http.StatusOK, + }, + }, + serviceBody: ServiceBody{ + APIName: "daleapi", + Documentation: []byte("\"docs\""), + Image: "abcde", + ImageContentType: "image/jpeg", + ResourceType: Oas2, + RestAPIID: "12345", + specHash: "abc1234", + originalSpecHash: "abc123", + specHashes: map[string]interface{}{ + "abc123": "daleapi-1", + }, + serviceContext: serviceContext{ + serviceAction: updateAPI, + }, + }, + expectedRevName: "daleapi-1", }, - }) - cloneServiceBody := serviceBody - // Normal Revision - client.processRevision(&cloneServiceBody) - assert.NotEqual(t, "", cloneServiceBody.serviceContext.revisionName) + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + if tc.skip { + return + } + // tests for updating existing revision + httpClient.SetResponses(tc.httpResponses) + + client.processRevision(&tc.serviceBody) + assert.NotEqual(t, "", tc.serviceBody.serviceContext.revisionName) + assert.Equal(t, tc.expectedRevName, tc.serviceBody.serviceContext.revisionName) + }) + } } func TestDeleteServiceByAPIID(t *testing.T) { diff --git a/pkg/apic/apiservicerevision.go b/pkg/apic/apiservicerevision.go index e6af85c26..235ca4c9d 100644 --- a/pkg/apic/apiservicerevision.go +++ b/pkg/apic/apiservicerevision.go @@ -7,9 +7,7 @@ import ( "errors" "fmt" "net/http" - "reflect" "regexp" - "sort" "strconv" "strings" "text/template" @@ -165,7 +163,15 @@ func (c *ServiceClient) getRevisionsIfUpdating(serviceBody *ServiceBody) ([]*man // checkAndUpdateExistingRevision checks if a revision with the same hash exists and updates tags if needed func (c *ServiceClient) checkAndUpdateExistingRevision(serviceBody *ServiceBody, apiServiceRevisions []*management.APIServiceRevision) (bool, error) { + // attempt to use the stripped spec hash revName, found := serviceBody.specHashes[serviceBody.specHash] + if !found && serviceBody.originalSpecHash != "" { + // check if the original spec hash matches an existing revision, + // this is to cover the case where the spec content has not changed since the last publish, + // but the hash has changed due to non-content related changes (e.g. stripping servers) + revName, found = serviceBody.specHashes[serviceBody.originalSpecHash] + } + if !found { return false, nil } @@ -205,24 +211,13 @@ func (c *ServiceClient) checkAndUpdateExistingRevision(serviceBody *ServiceBody, // different, return the updated tags func (c *ServiceClient) getUpdatedTagKeys(serviceBodyTags map[string]interface{}, revisionTags []string) []string { // Extract values from map and convert to []string - var mapValues []string - for _, v := range serviceBodyTags { - if strVal, ok := v.(string); ok { - mapValues = append(mapValues, strVal) - } - } - - // Sort both slices to allow unordered comparison - sort.Strings(mapValues) - sort.Strings(revisionTags) + tags := mapToTagsArray(serviceBodyTags, c.cfg.GetTagsToPublish()) // Compare - if reflect.DeepEqual(mapValues, revisionTags) { + if util.StringSlicesEqualUnordered(tags, revisionTags) { return []string{} // return empty string slice if equal } - // If not equal, return the keys from serviceBodyTags - tags := mapToTagsArray(serviceBodyTags, c.cfg.GetTagsToPublish()) return tags } diff --git a/pkg/apic/client.go b/pkg/apic/client.go index 91d2dd10d..0066fb07b 100644 --- a/pkg/apic/client.go +++ b/pkg/apic/client.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "slices" "strconv" "strings" "sync" @@ -870,7 +869,7 @@ func (c *ServiceClient) updateSpecORCreateResourceInstance(data *apiv1.ResourceI method = coreapi.PUT // check if either hash, tags or title have changed and mark for update - equalTags := slices.Equal(existingRI.GetTags(), data.GetTags()) + equalTags := util.StringSlicesEqualUnordered(existingRI.GetTags(), data.GetTags()) oldHash, _ := util.GetAgentDetailsValue(existingRI, defs.AttrSpecHash) newHash, _ := util.GetAgentDetailsValue(data, defs.AttrSpecHash) if oldHash == newHash && existingRI.Title == data.Title && equalTags { diff --git a/pkg/apic/servicebody.go b/pkg/apic/servicebody.go index 4e9bebf43..fa398b5bb 100644 --- a/pkg/apic/servicebody.go +++ b/pkg/apic/servicebody.go @@ -14,59 +14,61 @@ type APIKeyInfo struct { // ServiceBody - details about a service to create type ServiceBody struct { - NameToPush string - APIName string - RestAPIID string - PrimaryKey string - URL string - Stage string - StageDescriptor string - StageDisplayName string - Description string - Version string - AuthPolicy string - authPolicies []string - apiKeyInfo []APIKeyInfo - scopes map[string]string - SpecDefinition []byte - Documentation []byte - Tags map[string]interface{} - Image string - ImageContentType string - CreatedBy string - ResourceContentType string - ResourceType string - SubscriptionName string - APIUpdateSeverity string - State string - Status string - ServiceAttributes map[string]string - RevisionAttributes map[string]string - InstanceAttributes map[string]string - ServiceAgentDetails map[string]interface{} - InstanceAgentDetails map[string]interface{} - RevisionAgentDetails map[string]interface{} - serviceContext serviceContext - Endpoints []EndpointDefinition - UnstructuredProps *UnstructuredProperties - TeamName string - teamID string - credentialRequestPolicies []string - ardName string - uniqueARD bool - ignoreSpecBasesCreds bool - stripOASExtensions bool - specHash string - specVersion string - accessRequestDefinition *management.AccessRequestDefinition - specHashes map[string]interface{} // map of hash values to revision names - requestDefinitionsAllowed bool // used to validate if the instance can have request definitions or not. Use case example - v7 unpublished, remove request definitions - dataplaneType DataplaneType - isDesignDataplane bool - referencedServiceName string - referencedInstanceName string - logger log.FieldLogger - instanceLifecycle *management.ApiServiceInstanceLifecycle + NameToPush string + APIName string + RestAPIID string + PrimaryKey string + URL string + Stage string + StageDescriptor string + StageDisplayName string + Description string + Version string + AuthPolicy string + authPolicies []string + apiKeyInfo []APIKeyInfo + scopes map[string]string + SpecDefinition []byte + Documentation []byte + Tags map[string]interface{} + Image string + ImageContentType string + CreatedBy string + ResourceContentType string + ResourceType string + SubscriptionName string + APIUpdateSeverity string + State string + Status string + ServiceAttributes map[string]string + RevisionAttributes map[string]string + InstanceAttributes map[string]string + ServiceAgentDetails map[string]interface{} + InstanceAgentDetails map[string]interface{} + RevisionAgentDetails map[string]interface{} + serviceContext serviceContext + Endpoints []EndpointDefinition + UnstructuredProps *UnstructuredProperties + TeamName string + teamID string + credentialRequestPolicies []string + ardName string + uniqueARD bool + ignoreSpecBasesCreds bool + stripOASExtensions bool + stripOASServersBeforePublish bool + specHash string + specVersion string + accessRequestDefinition *management.AccessRequestDefinition + specHashes map[string]interface{} // map of hash values to revision names + requestDefinitionsAllowed bool // used to validate if the instance can have request definitions or not. Use case example - v7 unpublished, remove request definitions + dataplaneType DataplaneType + isDesignDataplane bool + referencedServiceName string + referencedInstanceName string + logger log.FieldLogger + instanceLifecycle *management.ApiServiceInstanceLifecycle + originalSpecHash string } // SetAccessRequestDefinitionName - set the name of the access request definition for this service body diff --git a/pkg/apic/servicebuilder.go b/pkg/apic/servicebuilder.go index 52d1abd81..c4bca3a8b 100644 --- a/pkg/apic/servicebuilder.go +++ b/pkg/apic/servicebuilder.go @@ -56,6 +56,7 @@ type ServiceBuilder interface { SetAccessRequestDefinitionName(accessRequestDefName string, isUnique bool) ServiceBuilder SetIgnoreSpecBasedCreds(ignore bool) ServiceBuilder SetStripOASExtensions(strip bool) ServiceBuilder + SetStripOASServersBeforePublish() ServiceBuilder SetUnstructuredType(assetType string) ServiceBuilder SetUnstructuredContentType(contentType string) ServiceBuilder @@ -379,6 +380,14 @@ func (b *serviceBodyBuilder) Build() (ServiceBody, error) { val.StripExtensions() } + if b.serviceBody.stripOASServersBeforePublish { + b.serviceBody.originalSpecHash = b.serviceBody.specHash + val.stripEndpoints() + b.serviceBody.SpecDefinition = val.GetSpecBytes() + newHash, _ := util.ComputeHash(val.GetSpecBytes()) + b.serviceBody.specHash = fmt.Sprintf("%v", newHash) + } + // only set ard name based on spec if not already set, use first auth we find if b.serviceBody.ardName != "" { return b.serviceBody, nil @@ -426,6 +435,11 @@ func (b *serviceBodyBuilder) SetIgnoreSpecBasedCreds(ignore bool) ServiceBuilder return b } +func (b *serviceBodyBuilder) SetStripOASServersBeforePublish() ServiceBuilder { + b.serviceBody.stripOASServersBeforePublish = true + return b +} + func (b *serviceBodyBuilder) SetStripOASExtensions(strip bool) ServiceBuilder { b.serviceBody.stripOASExtensions = strip return b diff --git a/pkg/apic/servicebuilder_test.go b/pkg/apic/servicebuilder_test.go index 93a081cde..6b73ea9f1 100644 --- a/pkg/apic/servicebuilder_test.go +++ b/pkg/apic/servicebuilder_test.go @@ -84,6 +84,7 @@ func TestServiceBodySetters(t *testing.T) { SetReferenceInstanceName("refInstance", "refEnv"). SetIgnoreSpecBasedCreds(true). SetStripOASExtensions(true). + SetStripOASServersBeforePublish(). SetInstanceLifecycle("stage", "active", ""). SetServiceEndpoints(ep) @@ -136,6 +137,7 @@ func TestServiceBodySetters(t *testing.T) { assert.Equal(t, "refEnv/refInstance", sb.GetReferenceInstanceName()) assert.Equal(t, true, sb.ignoreSpecBasesCreds) assert.Equal(t, true, sb.stripOASExtensions) + assert.Equal(t, true, sb.stripOASServersBeforePublish) instanceLifecycle := sb.GetInstanceLifeCycle() assert.NotNil(t, instanceLifecycle) assert.Equal(t, "stage", instanceLifecycle.Stage) @@ -222,7 +224,7 @@ func TestServiceBodyBuilderWithLargeSpec(t *testing.T) { "status": "active", } - sb, err = serviceBuilder. + serviceBuilder = serviceBuilder. SetDescription("A sample Petstore server based on OpenAPI 3.0"). SetAPIName("petstore"). SetAuthPolicy("pass-through"). @@ -230,8 +232,9 @@ func TestServiceBodyBuilderWithLargeSpec(t *testing.T) { SetState(PublishedState). SetServiceEndpoints(endpoints). SetTags(tags). - SetTeamName("pet-team"). - Build() + SetTeamName("pet-team") + + sb, err = serviceBuilder.Build() // Assertions assert.Nil(t, err) @@ -246,9 +249,17 @@ func TestServiceBodyBuilderWithLargeSpec(t *testing.T) { assert.Equal(t, endpoints, sb.Endpoints) assert.Equal(t, tags, sb.Tags) assert.NotEmpty(t, sb.SpecDefinition) + assert.Empty(t, sb.originalSpecHash) assert.Equal(t, Oas3, sb.ResourceType) // Verify spec size constraints assert.Less(t, len(sb.SpecDefinition), len(specBytes), "processed spec should be smaller than original") assert.Less(t, len(sb.SpecDefinition), tenMB, "spec should be under 10MB limit") + + sb, err = serviceBuilder. + SetStripOASServersBeforePublish(). + Build() + assert.Nil(t, err) + assert.NotNil(t, sb) + assert.NotEqual(t, sb.originalSpecHash, sb.specHash) } diff --git a/pkg/apic/specoas2processor.go b/pkg/apic/specoas2processor.go index 753d3d421..6e8553672 100644 --- a/pkg/apic/specoas2processor.go +++ b/pkg/apic/specoas2processor.go @@ -78,6 +78,13 @@ func (p *oas2SpecProcessor) GetEndpoints() ([]EndpointDefinition, error) { return endPoints, nil } +func (p *oas2SpecProcessor) stripEndpoints() { + // strip the endpoints from the spec, these will be added based on the API Service EndpointDefinitions + p.spec.BasePath = "" + p.spec.Host = "" + p.spec.Schemes = []string{} +} + func (p *oas2SpecProcessor) ParseAuthInfo() { authPolicies := []string{} keyInfo := []APIKeyInfo{} diff --git a/pkg/apic/specoas3processor.go b/pkg/apic/specoas3processor.go index f1dbd536d..a93b361f9 100644 --- a/pkg/apic/specoas3processor.go +++ b/pkg/apic/specoas3processor.go @@ -280,6 +280,11 @@ func (p *oas3SpecProcessor) StripExtensions() { } } +func (p *oas3SpecProcessor) stripEndpoints() { + // strip the endpoints from the spec, these will be added based on the API Service EndpointDefinitions + p.spec.Servers = []*openapi3.Server{} +} + func (p *oas3SpecProcessor) GetSpecBytes() []byte { s, _ := json.Marshal(p.spec) return s diff --git a/pkg/apic/specparser.go b/pkg/apic/specparser.go index 053d60c30..38e265538 100644 --- a/pkg/apic/specparser.go +++ b/pkg/apic/specparser.go @@ -57,6 +57,7 @@ type OasSpecProcessor interface { GetSpecBytes() []byte GetResourceType() string GetVersion() string + stripEndpoints() } // SpecResourceParser - diff --git a/pkg/util/util.go b/pkg/util/util.go index 60d223318..60fa3b01f 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -595,3 +595,21 @@ func IsInArray[K comparable](arr []K, val K) bool { } return false } + +func StringSlicesEqualUnordered(a, b []string) bool { + if len(a) != len(b) { + return false + } + + counts := make(map[string]int) + for _, item := range a { + counts[item]++ + } + for _, item := range b { + counts[item]-- + if counts[item] < 0 { + return false + } + } + return true +} diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index ad5d7984a..62664aab5 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -511,3 +511,12 @@ func TestGCMEcryptor(t *testing.T) { }) } } + +func TestStringSlicesEqualUnordered(t *testing.T) { + slice1 := []string{"foo", "bar"} + slice2 := []string{"bar", "foo"} + slice3 := []string{"foo", "baz"} + + assert.True(t, StringSlicesEqualUnordered(slice1, slice2)) + assert.False(t, StringSlicesEqualUnordered(slice1, slice3)) +}