APIGOV-22850 - one api service instance per stage (#486)

* APIGOV-22850 - only keep latest instance per stage

* APIGOV-22850 - handle migration to remove all but latest instance

* APIGOV-22850 - get previous revsion instance when updating

* APIGOV-22850 - add migration config settings

* APIGOV-22850 - rename component in logger

* APIGOV-22850 - fix test

* APIGOV-22850 - use mutex for test

* APIGOV-22850 - test config property

* APIGOV-22850 - fix lint

* APIGOV-22850 - test apisi migration module

* APIGOV-22850 - fix data race in test

* APIGOV-22850 - use proper url for updates to instance

* APIGOV-22850 - use CreateOrUpdate method

* APIGOV-22850 - set migration flags as complete on new services

* APIGOV-22850 - fix test

pkg/agent/cache/apiserviceinstance.go

@@ -10,7 +10,7 @@ import (
 func (c *cacheManager) AddAPIServiceInstance(resource *v1.ResourceInstance) {
 	defer c.setCacheUpdated(true)
 
-	c.instanceMap.Set(resource.Metadata.ID, resource)
+	c.instanceMap.SetWithSecondaryKey(resource.Metadata.ID, resource.Name, resource)
 }
 
 // GetAPIServiceInstanceKeys - returns keys for APIServiceInstance cache
@@ -36,6 +36,21 @@ func (c *cacheManager) GetAPIServiceInstanceByID(id string) (*v1.ResourceInstanc
 	return nil, err
 }
 
+// GetAPIServiceInstanceByName - returns resource from APIServiceInstance cache based on instance name
+func (c *cacheManager) GetAPIServiceInstanceByName(name string) (*v1.ResourceInstance, error) {
+	c.ApplyResourceReadLock()
+	defer c.ReleaseResourceReadLock()
+
+	item, err := c.instanceMap.GetBySecondaryKey(name)
+	if item != nil {
+		instance, ok := item.(*v1.ResourceInstance)
+		if ok {
+			return instance, nil
+		}
+	}
+	return nil, err
+}
+
 // DeleteAPIServiceInstance - remove APIServiceInstance resource from cache based on instance ID
 func (c *cacheManager) DeleteAPIServiceInstance(id string) error {
 	defer c.setCacheUpdated(true)

pkg/agent/cache/manager.go

@@ -38,6 +38,7 @@ type Manager interface {
 	AddAPIServiceInstance(resource *v1.ResourceInstance)
 	GetAPIServiceInstanceKeys() []string
 	GetAPIServiceInstanceByID(id string) (*v1.ResourceInstance, error)
+	GetAPIServiceInstanceByName(apiName string) (*v1.ResourceInstance, error)
 	DeleteAPIServiceInstance(id string) error
 	DeleteAllAPIServiceInstance()
 

pkg/agent/eventsync.go

@@ -35,7 +35,8 @@ func NewEventSync() (*EventSync, error) {
 		// add attribute migration to migrations
 		attributeMigration := migrate.NewAttributeMigration(agent.apicClient, agent.cfg)
 		ardMigration := migrate.NewArdMigration(agent.apicClient, agent.cfg)
-		migrations = append(migrations, attributeMigration, ardMigration)
+		apisiMigration := migrate.NewAPISIMigration(agent.apicClient, agent.cfg)
+		migrations = append(migrations, attributeMigration, ardMigration, apisiMigration)
 
 		if isMpEnabled {
 			// add marketplace migration to migrations

pkg/apic/apiservice.go

@@ -53,11 +53,21 @@ func (c *ServiceClient) buildAPIService(serviceBody *ServiceBody) *mv1a.APIServi
 	}
 
 	svcDetails := buildAgentDetailsSubResource(serviceBody, true, serviceBody.ServiceAgentDetails)
+	c.setMigrationFlags(svcDetails)
+
 	util.SetAgentDetails(svc, svcDetails)
 
 	return svc
 }
 
+func (c *ServiceClient) setMigrationFlags(svcDetails map[string]interface{}) {
+	svcDetails[defs.InstanceMigration] = defs.MigrationCompleted
+
+	if c.cfg.IsMarketplaceSubsEnabled() {
+		svcDetails[defs.MarketplaceMigration] = defs.MigrationCompleted
+	}
+}
+
 func (c *ServiceClient) getOwnerObject(serviceBody *ServiceBody, warning bool) (*v1.Owner, error) {
 	if id, found := c.getTeamFromCache(serviceBody.TeamName); found {
 		return &v1.Owner{

pkg/apic/apiservice_test.go

@@ -105,7 +105,6 @@ func TestCreateService(t *testing.T) {
 	apiSvc, err := client.PublishService(&cloneServiceBody)
 	assert.Nil(t, err)
 	assert.NotNil(t, apiSvc)
-	assert.Equal(t, &cloneServiceBody.serviceContext.revisionName, &cloneServiceBody.serviceContext.instanceName)
 	// this should fail
 	httpClient.SetResponses([]api.MockResponse{
 		{
@@ -274,6 +273,10 @@ func TestUpdateService(t *testing.T) {
 			FileName: "./testdata/apiservice.json", // for call to update the service subresource
 			RespCode: http.StatusOK,
 		},
+		{
+			FileName: "./testdata/apiservice.json", // for call to update the service subresource
+			RespCode: http.StatusOK,
+		},
 		{
 			FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision
 			RespCode: http.StatusOK,
@@ -309,7 +312,6 @@ func TestUpdateService(t *testing.T) {
 	apiSvc, err := client.PublishService(&cloneServiceBody)
 	assert.Nil(t, err)
 	assert.NotNil(t, apiSvc)
-	assert.Equal(t, &cloneServiceBody.serviceContext.revisionName, &cloneServiceBody.serviceContext.instanceName)
 
 	// tests for updating existing instance with same endpoint
 	httpClient.SetResponses([]api.MockResponse{
@@ -321,6 +323,10 @@ func TestUpdateService(t *testing.T) {
 			FileName: "./testdata/apiservice.json", // for call to update the service subresource
 			RespCode: http.StatusOK,
 		},
+		{
+			FileName: "./testdata/apiservice.json", // for call to update the service subresource
+			RespCode: http.StatusOK,
+		},
 		{
 			FileName: "./testdata/servicerevision.json", // for call to update the serviceRevision
 			RespCode: http.StatusOK,

pkg/apic/apiserviceinstance.go

@@ -3,7 +3,6 @@ package apic
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"net/http"
 	"strconv"
 
@@ -141,37 +140,24 @@ func (c *ServiceClient) processInstance(serviceBody *ServiceBody) error {
 		return err
 	}
 
-	var httpMethod string
-	var instance *mv1a.APIServiceInstance
+	// creating new instance
+	instance := c.buildAPIServiceInstance(serviceBody, getRevisionPrefix(serviceBody), endpoints)
 
-	instanceURL := c.cfg.GetInstancesURL()
-	instancePrefix := getRevisionPrefix(serviceBody)
-	instanceName := instancePrefix + "." + strconv.Itoa(serviceBody.serviceContext.revisionCount)
-
-	if serviceBody.serviceContext.revisionAction == addAPI {
-		httpMethod = http.MethodPost
-		instance = c.buildAPIServiceInstance(serviceBody, instanceName, endpoints)
-	}
-
-	if serviceBody.serviceContext.revisionAction == updateAPI {
-		httpMethod = http.MethodPut
-		instances, err := c.getRevisionInstances(instanceName, instanceURL)
+	if serviceBody.serviceContext.serviceAction == updateAPI {
+		prevInst, err := c.getLastInstance(serviceBody, c.createAPIServerURL(instance.GetKindLink()))
 		if err != nil {
 			return err
 		}
-		if len(instances) == 0 {
-			return fmt.Errorf("no instance found named '%s' for revision '%s'", instanceName, serviceBody.serviceContext.revisionName)
+
+		if prevInst != nil {
+			// updating existing instance
+			instance = c.updateAPIServiceInstance(serviceBody, prevInst, endpoints)
 		}
-		instanceURL = instanceURL + "/" + instanceName
-		instance = c.updateAPIServiceInstance(serviceBody, instances[0], endpoints)
 	}
 
-	buffer, err := json.Marshal(instance)
-	if err != nil {
-		return err
-	}
+	addSpecHashToResource(instance)
 
-	ri, err := c.executeAPIServiceAPI(httpMethod, instanceURL, buffer)
+	ri, err := c.CreateOrUpdateResource(instance)
 	if err != nil {
 		if serviceBody.serviceContext.serviceAction == addAPI {
 			_, rollbackErr := c.rollbackAPIService(serviceBody.serviceContext.serviceName)
@@ -199,7 +185,7 @@ func (c *ServiceClient) processInstance(serviceBody *ServiceBody) error {
 	}
 
 	c.caches.AddAPIServiceInstance(ri)
-	serviceBody.serviceContext.instanceName = instanceName
+	serviceBody.serviceContext.instanceName = instance.Name
 
 	return err
 }
@@ -233,13 +219,23 @@ func createInstanceEndpoint(endpoints []EndpointDefinition) ([]mv1a.ApiServiceIn
 	return endPoints, nil
 }
 
-func (c *ServiceClient) getRevisionInstances(name, url string) ([]*mv1a.APIServiceInstance, error) {
-	// Check if instances exist for the current revision.
-	queryParams := map[string]string{
-		"query": "name==" + name,
-	}
+func (c *ServiceClient) getLastInstance(serviceBody *ServiceBody, url string) (*mv1a.APIServiceInstance, error) {
+	// start from latest revision, find first instance
+	for i := serviceBody.serviceContext.revisionCount; i > 0; i-- {
+		queryParams := map[string]string{
+			"query": "metadata.references.name==" + getRevisionPrefix(serviceBody) + "." + strconv.Itoa(i),
+		}
 
-	return c.GetAPIServiceInstances(queryParams, url)
+		instances, err := c.GetAPIServiceInstances(queryParams, url)
+		if err != nil {
+			return nil, err
+		}
+
+		if len(instances) > 0 {
+			return instances[0], nil
+		}
+	}
+	return nil, nil
 }
 
 // GetAPIServiceInstanceByName - Returns the API service instance for specified name

pkg/apic/client.go

@@ -806,6 +806,8 @@ func (c *ServiceClient) updateSpecORCreateResourceInstance(data *v1.ResourceInst
 		existingRI, err = c.caches.GetAccessRequestDefinitionByName(data.Name)
 	case mv1a.CredentialRequestDefinitionGVK().Kind:
 		existingRI, err = c.caches.GetCredentialRequestDefinitionByName(data.Name)
+	case mv1a.APIServiceInstanceGVK().Kind:
+		existingRI, err = c.caches.GetAPIServiceInstanceByName(data.Name)
 	}
 
 	if err == nil && existingRI != nil {

pkg/apic/definitions/definitions.go

@@ -37,6 +37,7 @@ const (
 	ReferencesSubResource            = "references"
 	Subscription                     = "Subscription"
 	MarketplaceMigration             = "marketplace-migration"
+	InstanceMigration                = "instance-migration"
 )
 
 // market place provisioning migration

pkg/apic/util.go

@@ -0,0 +1,24 @@
+package apic
+
+import (
+	"fmt"
+
+	v1 "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/api/v1"
+	"github.com/Axway/agent-sdk/pkg/apic/definitions"
+	"github.com/Axway/agent-sdk/pkg/util"
+)
+
+func addSpecHashToResource(h v1.Interface) error {
+	ri, err := h.AsInstance()
+	if err != nil {
+		return err
+	}
+
+	hashInt, err := util.ComputeHash(ri.Spec)
+	if err != nil {
+		return err
+	}
+
+	util.SetAgentDetailsKey(h, definitions.AttrSpecHash, fmt.Sprintf("%v", hashInt))
+	return nil
+}

pkg/cmd/cmd_test.go

@@ -115,6 +115,7 @@ func TestRootCmdFlags(t *testing.T) {
 	assertStringSliceCmdFlag(t, rootCmd, "central.ssl.cipherSuites", "centralSslCipherSuites", corecfg.TLSDefaultCipherSuitesStringSlice(), "List of supported cipher suites, comma separated")
 	assertStringCmdFlag(t, rootCmd, "central.ssl.minVersion", "centralSslMinVersion", corecfg.TLSDefaultMinVersionString(), "Minimum acceptable SSL/TLS protocol version")
 	assertStringCmdFlag(t, rootCmd, "central.ssl.maxVersion", "centralSslMaxVersion", "0", "Maximum acceptable SSL/TLS protocol version")
+	assertBooleanCmdFlag(t, rootCmd, "central.migration.cleanInstances", "centralMigrationCleanInstances", false, "Set this to clean all but latest instance, per stage, within an API Service")
 
 	// Traceability Agent
 	rootCmd = NewRootCmd("Test", "TestRootCmd", nil, nil, corecfg.TraceabilityAgent)

pkg/config/centralconfig.go

@@ -124,6 +124,7 @@ type CentralConfig interface {
 	SetIsMarketplaceSubsEnabled(enabled bool)
 	IsMarketplaceSubsEnabled() bool
 	GetSingleURL() string
+	GetMigrationSettings() MigrationConfig
 }
 
 // CentralConfiguration - Structure to hold the central config
@@ -143,6 +144,7 @@ type CentralConfiguration struct {
 	APIServerVersion          string               `config:"apiServerVersion"`
 	TagsToPublish             string               `config:"additionalTags"`
 	AppendEnvironmentToTitle  bool                 `config:"appendEnvironmentToTitle"`
+	MigrationSettings         MigrationConfig      `config:"migration"`
 	Auth                      AuthConfig           `config:"auth"`
 	TLS                       TLSConfig            `config:"ssl"`
 	PollInterval              time.Duration        `config:"pollInterval"`
@@ -202,6 +204,7 @@ func NewCentralConfig(agentType AgentType) CentralConfig {
 				PageSize:  20,
 			},
 		},
+		MigrationSettings: newMigrationConfig(),
 	}
 }
 
@@ -212,6 +215,7 @@ func NewTestCentralConfig(agentType AgentType) CentralConfig {
 	config.URL = "https://central.com"
 	config.Environment = "environment"
 	config.Auth = newTestAuthConfig()
+	config.MigrationSettings = newTestMigrationConfig()
 	return config
 }
 
@@ -430,6 +434,11 @@ func (c *CentralConfiguration) GetAuthConfig() AuthConfig {
 	return c.Auth
 }
 
+// GetMigrationSettings - Returns the Migration Config
+func (c *CentralConfiguration) GetMigrationSettings() MigrationConfig {
+	return c.MigrationSettings
+}
+
 // GetTLSConfig - Returns the TLS Config
 func (c *CentralConfiguration) GetTLSConfig() TLSConfig {
 	return c.TLS
@@ -754,6 +763,7 @@ func AddCentralConfigProperties(props properties.Properties, agentType AgentType
 		props.AddStringProperty(pathAdditionalTags, "", "Additional Tags to Add to discovered APIs when publishing to Amplify Central")
 		props.AddBoolProperty(pathAppendEnvironmentToTitle, true, "When true API titles and descriptions will be appended with environment name")
 		AddSubscriptionConfigProperties(props)
+		AddMigrationConfigProperties(props)
 	}
 }
 
@@ -832,9 +842,11 @@ func ParseCentralConfig(props properties.Properties, agentType AgentType) (Centr
 		// set the notifications
 		subscriptionConfig := ParseSubscriptionConfig(props)
 		cfg.SubscriptionConfiguration = subscriptionConfig
+		cfg.MigrationSettings = ParseMigrationConfig(props)
 	}
 	return cfg, nil
 }
+
 func supportsTraceability(agentType AgentType) bool {
 	return agentType == TraceabilityAgent
 }

pkg/config/migrationconfig.go

@@ -0,0 +1,53 @@
+package config
+
+import "github.com/Axway/agent-sdk/pkg/cmd/properties"
+
+// MigrationConfig - Interface for migration settings config
+type MigrationConfig interface {
+	ShouldCleanInstances() bool
+	validate()
+}
+
+// MigrationSettings -
+type MigrationSettings struct {
+	CleanInstances bool
+}
+
+func newMigrationConfig() MigrationConfig {
+	return &MigrationSettings{
+		CleanInstances: false,
+	}
+}
+
+func newTestMigrationConfig() MigrationConfig {
+	return &MigrationSettings{
+		CleanInstances: true,
+	}
+}
+
+func (m *MigrationSettings) validate() {
+}
+
+// ShouldCleanInstances - returns the value fo CleanInstances
+func (m *MigrationSettings) ShouldCleanInstances() bool {
+	return m.CleanInstances
+}
+
+const (
+	pathCleanInstances = "central.migration.cleanInstances"
+)
+
+// AddMigrationConfigProperties - Adds the command properties needed for Migration Config
+func AddMigrationConfigProperties(props properties.Properties) {
+	props.AddBoolProperty(pathCleanInstances, false, "Set this to clean all but latest instance, per stage, within an API Service")
+}
+
+// ParseMigrationConfig - Parses the Migration Config values from the command line
+func ParseMigrationConfig(props properties.Properties) MigrationConfig {
+	migrationConfig := newMigrationConfig()
+	migrationSettings := migrationConfig.(*MigrationSettings)
+
+	migrationSettings.CleanInstances = props.BoolPropertyValue(pathCleanInstances)
+
+	return migrationSettings
+}

pkg/config/migrationconfig_test.go

@@ -0,0 +1,13 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMigrationConfig(t *testing.T) {
+	defConf := newMigrationConfig()
+
+	assert.False(t, defConf.ShouldCleanInstances())
+}