Skip to content

Conversation

@karrgov
Copy link
Contributor

@karrgov karrgov commented Dec 19, 2025

LMCROSSITXSADEPLOY-2301

…lity in order to support the collecting of secrets and sending them to the backend

LMCROSSITXSADEPLOY-2301

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] [--apps-start-timeout TIMEOUT] [--apps-stage-timeout TIMEOUT] [--apps-upload-timeout TIMEOUT] [--apps-task-execution-timeout TIMEOUT]` + util.UploadEnvHelpText,
(EXPERIMENTAL) Deploy a multi-target app archive referenced by a remote URL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why experimental here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must have added it accidentally - fixed. I have just put it for the new flag, since we have talked about the feature being presented as experimental.


var jsonObject map[string]interface{}

err2 := json.Unmarshal([]byte(envValue), &jsonObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you reuse err?

Copy link
Contributor Author

@karrgov karrgov Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that reusing the same err is not recommended- I will just change the names and make them more descriptive instead of a plain err2 :)

Comment on lines 358 to 360
if GetBoolOpt(requireSecureParameters, flags) {
// Collect special ENVs: __MTA___<name>, __MTA_JSON___<name>, __MTA_CERT___<name>
parameters, err := secure_parameters.CollectFromEnv("__MTA")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check happens after the upload of the mtar. Could be optimized so the validation is earlier to avoid unnecessary upload in case of some failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the suggestion, fixed :)

return result, nil
}

func BuildSecureExtension(parameters map[string]ParameterValue, mtaID string, schemaVersion string) ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract in a separate file

Comment on lines 127 to 129
if schemaVersion == "" {
schemaVersion = "3.3"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not hardcode versions in client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :)

Comment on lines 140 to 145
switch currentParameterValue.Type {
case typeJSON:
parametersDescriptor[name] = currentParameterValue.ObjectContent
default:
parametersDescriptor[name] = currentParameterValue.StringContent
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add a new method on ParameterValue and use it here (currentParameterValue.getValue())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks for the suggestion :)

func CollectFromEnv(prefix string) (map[string]ParameterValue, error) {
plainValue := prefix + "___"
jsonValue := prefix + "_JSON___"
certificateValue := prefix + "_CERT___" //X509value beacuse the certiciates are of type X509 (should be renamed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve or remove the comment description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks

return nil
}

func CollectFromEnv(prefix string) (map[string]ParameterValue, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is quite long, try to shorten it, split in functions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed :)

ObjectContent map[string]interface{}
}

func nameDuplicated(name, prefix string, result map[string]ParameterValue) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to something including "validate"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :)

envName := nameValuePair[:equalsIndex]
envValue := nameValuePair[equalsIndex+1:]

var name string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need to define here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise I have to define it in each separate case

Comment on lines 474 to 480
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)
}
stringTable := strings.Join(servicesOutput, "\n")
return findServiceName(stringTable, userProvidedServiceName), nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call the v3/services here instead a command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely fixed, thanks for the suggestion :)

return true, encryptionKey, nil
}

func getRandomEncryptionKey() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there some library that can do this for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could not manage to find anything in particular - saw that many use the base64 encoding which I tried but since for every 3 bytes it spits out 4 characters - I was getting 43 chars out of my 32 bytes. Also since I wanted the constraint of having only alphanumerical values and not any / or + which base64 brings, I decided the use the explicit "alphabet" of allowed symbols and then just randomly choose each character from it.


userProvidedServiceName := getUpsName(mtaId, namespace)

isUpsCreated, _, err := c.validateUpsExistsOrElseCreateIt(userProvidedServiceName, "v1")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the idea of:
"keyId": "v1"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have discussed it during the multiapps-controller changes :)

Comment on lines +467 to +471
for i := range encryptionKeyBytes {
encryptionKeyBytes[i] = alphabet[int(encryptionKeyBytes[i]&63)]
}

return string(encryptionKeyBytes), nil
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really creates a proper 256 bits keys? Why not something like:

func generateAES256Key() ([]byte, error) {
key := make([]byte, 32) // 256 bits
_, err := rand.Read(key)
if err != nil {
return nil, err
}
return string([]byte(fmt.Sprintf("%x", key))), nil
}

Why is alphabet needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does. I tried doing it like that myself in the beginning, but the problem was that the argument in the Sprintf method determines the encoding used, thus giving different lenghts as a result. Using the fmt.Sprinf("%x", key) with the "x" argument, Go does a base16 encoding which actually converts the 32 generated bytes into 64 characters. The base16 encoding doubles the size of the byte input when it is converted to a string output, since base16 uses only the letters from 'a' to 'f' and 1 through 9, 16 symbols altogether, and since one byte can be between 0-255 - in order to represent all the 256 different values we need to have for each of the 16 unique symbols, 16 more - meaning 16x16 = 16^2 = 256

LMCROSSITXSADEPLOY-2301
LMCROSSITXSADEPLOY-2301
Type typeOfValue
StringContent string
JSONContent interface{}
//ObjectContent map[string]interface{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment if not needed.

}
}

func TestCollectFromEnvWhenDuplciateNames(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix typo

"encryptionKey": encryptionKey,
"keyId": keyID,
}
jsonBody, _ := json.Marshal(upsCredentials)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the error from json.Marshal ignored here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants