Adding fixes after review

LMCROSSITXSADEPLOY-2301

Author: karrgov

clients/cfrestclient/cloud_foundry_operations_extended.go

@@ -10,4 +10,5 @@ type CloudFoundryOperationsExtended interface {
 	GetApplicationRoutes(appGuid string) ([]models.ApplicationRoute, error)
 	GetServiceInstances(mtaId, mtaNamespace, spaceGuid string) ([]models.CloudFoundryServiceInstance, error)
 	GetServiceBindings(serviceName string) ([]models.ServiceBinding, error)
+	GetServiceInstanceByName(serviceName, spaceGuid string) (models.CloudFoundryServiceInstance, error)
 }

clients/cfrestclient/fakes/fake_cloud_foundry_client.go

@@ -34,3 +34,7 @@ func (f FakeCloudFoundryClient) GetServiceInstances(mtaId, mtaNamespace, spaceGu
 func (f FakeCloudFoundryClient) GetServiceBindings(serviceName string) ([]models.ServiceBinding, error) {
 	return f.ServiceBindings, f.ServiceBindingsErr
 }
+
+func (f FakeCloudFoundryClient) GetServiceInstanceByName(serviceName, spaceGuid string) (models.CloudFoundryServiceInstance, error) {
+	return f.Services[0], f.ServiceBindingsErr
+}

clients/cfrestclient/resilient/resilient_rest_cloud_foundry_client_extended.go

@@ -1,9 +1,10 @@
 package resilient
 
 import (
+	"time"
+
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/cfrestclient"
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
-	"time"
 )
 
 type ResilientCloudFoundryRestClient struct {
@@ -46,6 +47,12 @@ func (c ResilientCloudFoundryRestClient) GetServiceBindings(serviceName string)
 	}, c.MaxRetriesCount, c.RetryInterval)
 }
 
+func (c ResilientCloudFoundryRestClient) GetServiceInstanceByName(serviceName, spaceGuid string) (models.CloudFoundryServiceInstance, error) {
+	return retryOnError(func() (models.CloudFoundryServiceInstance, error) {
+		return c.CloudFoundryRestClient.GetServiceInstanceByName(serviceName, spaceGuid)
+	}, c.MaxRetriesCount, c.RetryInterval)
+}
+
 func retryOnError[T any](operation func() (T, error), retries int, retryInterval time.Duration) (T, error) {
 	result, err := operation()
 	for shouldRetry(retries, err) {

clients/cfrestclient/rest_cloud_foundry_client_extended.go

@@ -114,6 +114,27 @@ func (c CloudFoundryRestClient) GetServiceBindings(serviceName string) ([]models
 	return getPaginatedResourcesWithIncluded(getServiceBindingsUrl, token, c.isSslDisabled, buildServiceBinding)
 }
 
+func (c CloudFoundryRestClient) GetServiceInstanceByName(serviceName, spaceGuid string) (models.CloudFoundryServiceInstance, error) {
+	token, err := c.cliConn.AccessToken()
+	if err != nil {
+		return models.CloudFoundryServiceInstance{}, fmt.Errorf("failed to retrieve access token: %s", err)
+	}
+	apiEndpoint, _ := c.cliConn.ApiEndpoint()
+
+	getServicesUrl := fmt.Sprintf("%s/%sservice_instances?names=%s&space_guids=%s",
+		apiEndpoint, cfBaseUrl, serviceName, spaceGuid)
+	services, err := getPaginatedResourcesWithIncluded(getServicesUrl, token, c.isSslDisabled, buildServiceInstance)
+	if err != nil {
+		return models.CloudFoundryServiceInstance{}, err
+	}
+	if len(services) == 0 {
+		return models.CloudFoundryServiceInstance{}, fmt.Errorf("service instance not found")
+	}
+
+	resultService := services[0]
+	return resultService, nil
+}
+
 func getPaginatedResources[T any](url, token string, isSslDisabled bool) ([]T, error) {
 	var result []T
 	for url != "" {

commands/blue_green_deploy_command.go

@@ -20,7 +20,7 @@ type BlueGreenDeployCommand struct {
 // NewBlueGreenDeployCommand creates a new BlueGreenDeployCommand.
 func NewBlueGreenDeployCommand() *BlueGreenDeployCommand {
 	baseCmd := &BaseCommand{flagsParser: deployCommandLineArgumentsParser{}, flagsValidator: deployCommandFlagsValidator{}}
-	deployCmd := &DeployCommand{baseCmd, blueGreenDeployProcessParametersSetter(), &blueGreenDeployCommandProcessTypeProvider{}, os.Stdin, 30 * time.Second}
+	deployCmd := &DeployCommand{baseCmd, blueGreenDeployProcessParametersSetter(), &blueGreenDeployCommandProcessTypeProvider{}, os.Stdin, 30 * time.Second, nil}
 	bgDeployCmd := &BlueGreenDeployCommand{deployCmd}
 	baseCmd.Command = bgDeployCmd
 	return bgDeployCmd

commands/deploy_command.go

@@ -20,6 +20,8 @@ import (
 	"code.cloudfoundry.org/cli/v8/cf/terminal"
 	"code.cloudfoundry.org/cli/v8/plugin"
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/baseclient"
+	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/cfrestclient"
+	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/cfrestclient/resilient"
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/models"
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/clients/mtaclient"
 	"github.com/cloudfoundry-incubator/multiapps-cli-plugin/commands/retrier"
@@ -91,16 +93,23 @@ type DeployCommand struct {
 
 	FileUrlReader      fs.File
 	FileUrlReadTimeout time.Duration
+	CfClient           cfrestclient.CloudFoundryOperationsExtended
 }
 
 // NewDeployCommand creates a new deploy command.
 func NewDeployCommand() *DeployCommand {
 	baseCmd := &BaseCommand{flagsParser: deployCommandLineArgumentsParser{}, flagsValidator: deployCommandFlagsValidator{}}
-	deployCmd := &DeployCommand{baseCmd, deployProcessParametersSetter(), &deployCommandProcessTypeProvider{}, os.Stdin, 30 * time.Second}
+	deployCmd := &DeployCommand{baseCmd, deployProcessParametersSetter(), &deployCommandProcessTypeProvider{}, os.Stdin, 30 * time.Second, nil}
 	baseCmd.Command = deployCmd
 	return deployCmd
 }
 
+func (c *DeployCommand) Initialize(name string, cliConnection plugin.CliConnection) {
+	c.BaseCommand.Initialize(name, cliConnection)
+	delegate := cfrestclient.NewCloudFoundryRestClient(cliConnection)
+	c.CfClient = resilient.NewResilientCloudFoundryClient(delegate, maxRetriesCount, retryIntervalInSeconds)
+}
+
 // GetPluginCommand returns the plugin command details
 func (c *DeployCommand) GetPluginCommand() plugin.Command {
 	return plugin.Command{
@@ -114,7 +123,7 @@ func (c *DeployCommand) GetPluginCommand() plugin.Command {
    Perform action on an active deploy operation
    cf deploy -i OPERATION_ID -a ACTION [-u URL]
 
-   (EXPERIMENTAL) Deploy a multi-target app archive referenced by a remote URL
+   Deploy a multi-target app archive referenced by a remote URL
    <write MTA archive URL to STDOUT> | cf deploy [-e EXT_DESCRIPTOR[,...]] [-t TIMEOUT] [--version-rule VERSION_RULE] [-u MTA_CONTROLLER_URL] [--retries RETRIES] [--no-start] [--namespace NAMESPACE] [--apply-namespace-app-names true/false] [--apply-namespace-service-names true/false] [--apply-namespace-app-routes true/false] [--apply-namespace-as-suffix true/false ] [--delete-services] [--delete-service-keys] [--delete-service-brokers] [--keep-files] [--no-restart-subscribed-apps] [--do-not-fail-on-missing-permissions] [--abort-on-error] [--strategy STRATEGY] [--skip-testing-phase] [--skip-idle-start] [require-secure-parameters] [--apps-start-timeout TIMEOUT] [--apps-stage-timeout TIMEOUT] [--apps-upload-timeout TIMEOUT] [--apps-task-execution-timeout TIMEOUT]` + util.UploadEnvHelpText,
 
 			Options: map[string]string{
@@ -150,7 +159,7 @@ func (c *DeployCommand) GetPluginCommand() plugin.Command {
 				util.GetShortOption(taskExecutionTimeoutOpt):                    "Task execution timeout in seconds",
 				util.CombineFullAndShortParameters(startTimeoutOpt, timeoutOpt): "Start app timeout in seconds",
 				util.GetShortOption(shouldBackupPreviousVersionOpt):             "(EXPERIMENTAL) (STRATEGY: BLUE-GREEN, INCREMENTAL-BLUE-GREEN) Backup previous version of applications, use new cli command \"rollback-mta\" to rollback to the previous version",
-				util.GetShortOption(requireSecureParameters):                    "Pass secrets to the deploy service in a secure way",
+				util.GetShortOption(requireSecureParameters):                    "(EXPERIMENTAL) Pass secrets to the deploy service in a secure way",
 			},
 		},
 	}
@@ -278,6 +287,7 @@ func (c *DeployCommand) executeInternal(positionalArgs []string, dsHost string,
 	sequentialUpload := conf.GetUploadChunksSequentially()
 	disableProgressBar := conf.GetDisableUploadProgressBar()
 	fileUploader := NewFileUploader(mtaClient, namespace, uploadChunkSize, sequentialUpload, disableProgressBar)
+	var yamlBytes []byte
 
 	if isUrl {
 		var fileId string
@@ -328,6 +338,41 @@ func (c *DeployCommand) executeInternal(positionalArgs []string, dsHost string,
 			return Failure
 		}
 
+		if GetBoolOpt(requireSecureParameters, flags) {
+			// Collect special ENVs: __MTA___<name>, __MTA_JSON___<name>, __MTA_CERT___<name>
+			parameters, err := secure_parameters.CollectFromEnv("__MTA")
+			if err != nil {
+				ui.Failed("Secure parameters error: %s", err)
+				return Failure
+			}
+
+			if len(parameters) == 0 {
+				ui.Failed("No secure parameters found in environment. Set variables like __MTA___<name>, __MTA_JSON___<name>, or __MTA_CERT___<name>.")
+				return Failure
+			}
+
+			userProvidedServiceName := getUpsName(mtaId, namespace)
+
+			isUpsCreated, _, err := c.validateUpsExistsOrElseCreateIt(userProvidedServiceName, "v1")
+			if err != nil {
+				ui.Failed("Could not ensure user-provided service %s: %v", userProvidedServiceName, err)
+				return Failure
+			}
+
+			if isUpsCreated {
+				ui.Say("Created user-provided service %s for secure parameters.", terminal.EntityNameColor(userProvidedServiceName))
+			} else {
+				ui.Say("Using existing user-provided service %s for secure parameters.", terminal.EntityNameColor(userProvidedServiceName))
+			}
+
+			schemaVer := descriptor.SchemaVersion
+			yamlBytes, err = secure_parameters.BuildSecureExtension(parameters, mtaId, schemaVer)
+			if err != nil {
+				ui.Failed("Could not build secure extension: %s", err)
+				return Failure
+			}
+		}
+
 		// Upload the MTA archive file
 		uploadedArchivePartIds, uploadStatus = c.uploadFiles([]string{mtaArchivePath}, fileUploader)
 		if uploadStatus == Failure {
@@ -356,39 +401,6 @@ func (c *DeployCommand) executeInternal(positionalArgs []string, dsHost string,
 	}
 
 	if GetBoolOpt(requireSecureParameters, flags) {
-		// Collect special ENVs: __MTA___<name>, __MTA_JSON___<name>, __MTA_CERT___<name>
-		parameters, err := secure_parameters.CollectFromEnv("__MTA")
-		if err != nil {
-			ui.Failed("Secure parameters error: %s", err)
-			return Failure
-		}
-
-		if len(parameters) == 0 {
-			ui.Failed("No secure parameters found in environment. Set variables like __MTA___<name>, __MTA_JSON___<name>, or __MTA_CERT___<name>.")
-			return Failure
-		}
-
-		userProvidedServiceName := getUpsName(mtaId, namespace)
-
-		isUpsCreated, _, err := c.validateUpsExistsOrElseCreateIt(userProvidedServiceName, "v1")
-		if err != nil {
-			ui.Failed("Could not ensure user-provided service %s: %v", userProvidedServiceName, err)
-			return Failure
-		}
-
-		if isUpsCreated {
-			ui.Say("Created user-provided service %s for secure parameters.", terminal.EntityNameColor(userProvidedServiceName))
-		} else {
-			ui.Say("Using existing user-provided service %s for secure parameters.", terminal.EntityNameColor(userProvidedServiceName))
-		}
-
-		schemaVer := ""
-		yamlBytes, err := secure_parameters.BuildSecureExtension(parameters, mtaId, schemaVer)
-		if err != nil {
-			ui.Failed("Could not build secure extension: %s", err)
-			return Failure
-		}
-
 		secureFileID, err := fileUploader.UploadBytes("__mta.secure.mtaext", yamlBytes)
 		if err != nil {
 			ui.Failed("Could not upload secure extension: %s", err)
@@ -472,26 +484,21 @@ func getRandomEncryptionKey() (string, error) {
 }
 
 func (c *DeployCommand) doesUpsExist(userProvidedServiceName string) (bool, error) {
-	servicesOutput, err := c.cliConnection.CliCommandWithoutTerminalOutput("services")
-	if err != nil {
-		return false, fmt.Errorf("Error while checking if the UPS for secure encryption exists: %w", err)
+	space, errSpace := c.cliConnection.GetCurrentSpace()
+	if errSpace != nil {
+		return false, fmt.Errorf("Cannot determine the current space")
 	}
-	stringTable := strings.Join(servicesOutput, "\n")
-	return findServiceName(stringTable, userProvidedServiceName), nil
-}
+	spaceGuid := space.Guid
 
-func findServiceName(servicesOutput, userProvidedServiceName string) bool {
-	userProvidedServiceNameToLower := strings.ToLower(userProvidedServiceName)
-	for _, currentLine := range strings.Split(servicesOutput, "\n") {
-		fields := strings.Fields(currentLine)
-		if len(fields) == 0 {
-			continue
-		}
-		if strings.ToLower(fields[0]) == userProvidedServiceNameToLower {
-			return true
+	_, errServiceInstance := c.CfClient.GetServiceInstanceByName(userProvidedServiceName, spaceGuid)
+	if errServiceInstance != nil {
+		if errServiceInstance.Error() == "service instance not found" {
+			return false, nil
 		}
+		return false, fmt.Errorf("Error while checking if the UPS for secure encryption exists: %w", errServiceInstance)
 	}
-	return false
+
+	return true, nil
 }
 
 func parseMtaArchiveArgument(rawMtaArchive interface{}) (bool, string) {

secure_parameters/secure_builder.go

@@ -0,0 +1,35 @@
+package secure_parameters
+
+import (
+	"errors"
+
+	"gopkg.in/yaml.v3"
+)
+
+func BuildSecureExtension(parameters map[string]ParameterValue, mtaID string, schemaVersion string) ([]byte, error) {
+	if len(parameters) == 0 {
+		return nil, errors.New("no secure parameters collected")
+	}
+
+	if mtaID == "" {
+		return nil, errors.New("mtaID is required for the secure extension descriptor's field 'extends'")
+	}
+
+	if schemaVersion == "" {
+		return nil, errors.New("schemaVersion is required for the secure extension descriptor")
+	}
+
+	secureExtensionDescriptor := map[string]interface{}{
+		"_schema-version": schemaVersion,
+		"ID":              "__mta.secure",
+		"extends":         mtaID,
+		"parameters":      map[string]interface{}{},
+	}
+
+	parametersDescriptor := secureExtensionDescriptor["parameters"].(map[string]interface{})
+	for name, currentParameterValue := range parameters {
+		parametersDescriptor[name] = getValue(&currentParameterValue)
+	}
+
+	return yaml.Marshal(secureExtensionDescriptor)
+}

secure_parameters/secure_parameters_test.go

@@ -44,12 +44,17 @@ func TestCollectFromEnv(t *testing.T) {
 		t.Fatalf("The value of 'fakeJson' key is not correct")
 	}
 
-	if firstJsonValue, ok := jsonValue.ObjectContent["a"].(float64); !ok || firstJsonValue != 1 {
-		t.Fatalf("The first value of the json is not what it should be: %v", jsonValue.ObjectContent["a"])
+	castedValue, ok := jsonValue.JSONContent.(map[string]interface{})
+	if !ok {
+		t.Fatal("fakeJson is not an Object")
+	}
+
+	if firstJsonValue, ok := castedValue["a"].(float64); !ok || firstJsonValue != 1 {
+		t.Fatalf("The first value of the json is not what it should be: %v", castedValue["a"])
 	}
 
-	if jsonValue.ObjectContent["b"] != "secretValueJson" {
-		t.Fatalf("The second value of the json is not what it should be: %v", jsonValue.ObjectContent["b"])
+	if castedValue["b"] != "secretValueJson" {
+		t.Fatalf("The second value of the json is not what it should be: %v", castedValue["b"])
 	}
 
 	certificateValue, ok := resultToTest["fakeCertificate"]
@@ -121,7 +126,7 @@ func TestCollectFromEnvWhenDifferentPrefix(t *testing.T) {
 func TestBuildSecureExtension(t *testing.T) {
 	parameters := map[string]ParameterValue{
 		"password":        {Type: typeString, StringContent: "secretValue"},
-		"fakeJson":        {Type: typeJSON, ObjectContent: map[string]interface{}{"secretParameterFirst": "secretValueOne", "secretParameterSecond": "secretValueTwo"}},
+		"fakeJson":        {Type: typeJSON, JSONContent: map[string]interface{}{"secretParameterFirst": "secretValueOne", "secretParameterSecond": "secretValueTwo"}},
 		"fakeCertificate": {Type: typeMultiline, StringContent: "-----BEGIN CERTIFICATE-----\nMIBgNVBAYTAPXwBc63heW9WrP3qnDEm+UZE4V0Au7OWnOeiobq\n-----END CERTIFICATE-----\n"},
 	}