pingidentity / pingcli

Apache License 2.0
0 stars 0 forks source link

PDI-2056: Add Defaults to Usage Output #18

Closed erikostien-pingidentity closed 1 week ago

erikostien-pingidentity commented 3 weeks ago
patrickcping commented 3 weeks ago

Potential problem:

➜  pingcli git:(PDI-2056) go run ./ completion zsh
Failed to validate Ping CLI configuration - Failure
Fatal: failed to validate Ping CLI configuration: invalid configuration key(s) found in profile pingfederate-terraform-uat: color
Must use one of: activeProfile, description, export.format, export.outputDirectory, export.overwrite, export.pingone.environmentID, export.services, noColor, outputFormat, request.accessToken, request.accessTokenExpiry, request.service, service.pingfederate.adminAPIPath, service.pingfederate.authentication.accessTokenAuth.accessToken, service.pingfederate.authentication.basicAuth.password, service.pingfederate.authentication.basicAuth.username, service.pingfederate.authentication.clientCredentialsAuth.clientID, service.pingfederate.authentication.clientCredentialsAuth.clientSecret, service.pingfederate.authentication.clientCredentialsAuth.scopes, service.pingfederate.authentication.clientCredentialsAuth.tokenURL, service.pingfederate.authentication.type, service.pingfederate.caCertificatePemFiles, service.pingfederate.httpsHost, service.pingfederate.insecureTrustAllTLS, service.pingfederate.xBypassExternalValidationHeader, service.pingone.authentication.type, service.pingone.authentication.worker.clientID, service.pingone.authentication.worker.clientSecret, service.pingone.authentication.worker.environmentID, service.pingone.regionCode
exit status 1

This probably needs something to remove color from all profiles when nocolor is set, or, the param switch is --no-color but the config file remains color

patrickcping commented 3 weeks ago

Is there a possibility of removing the --profile-name param on config set and config get commands, as --profile could do the same job?

erikostien-pingidentity commented 3 weeks ago

Potential problem:

➜  pingcli git:(PDI-2056) go run ./ completion zsh
Failed to validate Ping CLI configuration - Failure
Fatal: failed to validate Ping CLI configuration: invalid configuration key(s) found in profile pingfederate-terraform-uat: color
Must use one of: activeProfile, description, export.format, export.outputDirectory, export.overwrite, export.pingone.environmentID, export.services, noColor, outputFormat, request.accessToken, request.accessTokenExpiry, request.service, service.pingfederate.adminAPIPath, service.pingfederate.authentication.accessTokenAuth.accessToken, service.pingfederate.authentication.basicAuth.password, service.pingfederate.authentication.basicAuth.username, service.pingfederate.authentication.clientCredentialsAuth.clientID, service.pingfederate.authentication.clientCredentialsAuth.clientSecret, service.pingfederate.authentication.clientCredentialsAuth.scopes, service.pingfederate.authentication.clientCredentialsAuth.tokenURL, service.pingfederate.authentication.type, service.pingfederate.caCertificatePemFiles, service.pingfederate.httpsHost, service.pingfederate.insecureTrustAllTLS, service.pingfederate.xBypassExternalValidationHeader, service.pingone.authentication.type, service.pingone.authentication.worker.clientID, service.pingone.authentication.worker.clientSecret, service.pingone.authentication.worker.environmentID, service.pingone.regionCode
exit status 1

This probably needs something to remove color from all profiles when nocolor is set, or, the param switch is --no-color but the config file remains color

This is simply a validation step on pingcli startup. It informs the user that their config file is using a config key that is not recognized/does not exist. While this is a breaking change, since this is before beta, I think this is fine to add in without additional migration logic.

erikostien-pingidentity commented 3 weeks ago

Is there a possibility of removing the --profile-name param on config set and config get commands, as --profile could do the same job?

This is possible, but it will remove a subtlety here. For example in the following niche case, if a user specifies '--profile A' to output tool responses as json, but the user wants to modify --profile-name B, this would no longer be possible without additionally using the --output-format flag.

Since this is niche, I don't think there is an issue removing this capability. @patrickcping Thoughts?

patrickcping commented 3 weeks ago

Is there a possibility of removing the --profile-name param on config set and config get commands, as --profile could do the same job?

This is possible, but it will remove a subtlety here. For example in the following niche case, if a user specifies '--profile A' to output tool responses as json, but the user wants to modify --profile-name B, this would no longer be possible without additionally using the --output-format flag.

Since this is niche, I don't think there is an issue removing this capability. @patrickcping Thoughts?

I didn't consider that case, good spot. I agree it is niche though, let's flatten to --profile and remove that capability, on the basis that if users are setting a profile value on profile "A", they're also invoking the runtime config of that profile at the same time

patrickcping commented 3 weeks ago

Potential problem:

➜  pingcli git:(PDI-2056) go run ./ completion zsh
Failed to validate Ping CLI configuration - Failure
Fatal: failed to validate Ping CLI configuration: invalid configuration key(s) found in profile pingfederate-terraform-uat: color
Must use one of: activeProfile, description, export.format, export.outputDirectory, export.overwrite, export.pingone.environmentID, export.services, noColor, outputFormat, request.accessToken, request.accessTokenExpiry, request.service, service.pingfederate.adminAPIPath, service.pingfederate.authentication.accessTokenAuth.accessToken, service.pingfederate.authentication.basicAuth.password, service.pingfederate.authentication.basicAuth.username, service.pingfederate.authentication.clientCredentialsAuth.clientID, service.pingfederate.authentication.clientCredentialsAuth.clientSecret, service.pingfederate.authentication.clientCredentialsAuth.scopes, service.pingfederate.authentication.clientCredentialsAuth.tokenURL, service.pingfederate.authentication.type, service.pingfederate.caCertificatePemFiles, service.pingfederate.httpsHost, service.pingfederate.insecureTrustAllTLS, service.pingfederate.xBypassExternalValidationHeader, service.pingone.authentication.type, service.pingone.authentication.worker.clientID, service.pingone.authentication.worker.clientSecret, service.pingone.authentication.worker.environmentID, service.pingone.regionCode
exit status 1

This probably needs something to remove color from all profiles when nocolor is set, or, the param switch is --no-color but the config file remains color

This is simply a validation step on pingcli startup. It informs the user that their config file is using a config key that is not recognized/does not exist. While this is a breaking change, since this is before beta, I think this is fine to add in without additional migration logic.

Assuming this will break users who've previously installed the alpha build (there are some). How do we prevent errors for those users?

Edit: alpha users were using PingCTL - this wouldn't be in issue