Skip to content

[App Config] Fix #25325: az appconfig kv export: Fix the bug for exporting special characters in .properties file#25700

Closed
SaurabhSharma-MSFT wants to merge 3 commits intoAzure:devfrom
SaurabhSharma-MSFT:appconfig-fix
Closed

[App Config] Fix #25325: az appconfig kv export: Fix the bug for exporting special characters in .properties file#25700
SaurabhSharma-MSFT wants to merge 3 commits intoAzure:devfrom
SaurabhSharma-MSFT:appconfig-fix

Conversation

@SaurabhSharma-MSFT
Copy link
Copy Markdown
Member

Related command
az appconfig kv export -d file --path myConfig.properties --format properties

Description
Fix kv export cmdlet to export special characters in the exported .properties file.
Fixes GitHub Issue 25325

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Mar 6, 2023

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.12
️✔️3.9
️✔️computefleet
️✔️latest
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.12
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.12
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.12
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.12
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.12
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9

@ghost ghost requested review from wangzelin007 and yonzhan March 6, 2023 23:44
@ghost ghost assigned zhoxing-ms Mar 6, 2023
@ghost ghost added this to the March 2023 (2023-04-04) milestone Mar 6, 2023
@ghost ghost added App Configuration Auto-Assign Auto assign by bot labels Mar 6, 2023
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Mar 7, 2023

appconfig

@zhoxing-ms zhoxing-ms changed the title {appconfig} Fix kv export cmdlet to export special characters in .properties file {App config} Fix kv export cmdlet to export special characters in .properties file Mar 28, 2023
@zhoxing-ms zhoxing-ms changed the title {App config} Fix kv export cmdlet to export special characters in .properties file {App Config} Fix kv export cmdlet to export special characters in .properties file Mar 28, 2023
@zhoxing-ms zhoxing-ms changed the title {App Config} Fix kv export cmdlet to export special characters in .properties file [App Config] Fix #25325: az appconfig kv export: Fix the bug for exporting special characters in .properties file Mar 28, 2023
zhoxing-ms
zhoxing-ms previously approved these changes Mar 28, 2023
@zhoxing-ms
Copy link
Copy Markdown
Contributor

@albertofori Could you please help review this PR?

@albertofori
Copy link
Copy Markdown
Contributor

albertofori commented Mar 31, 2023

Hello @SaurabhSharma-MSFT please the current behaviour is expected based on the guidance on the documentation of properties file shown here.

In azure app configuration, we allow leading and trailing whitespaces and the characters that are considered "special" in the .properties format (!, #, etc) for keys, labels, values, etc.
Not escaping these values will result in them being parsed differently in java. Eg. without escaping special characters, a key that starts with the "#" or "!" character will have the whole line treated as a comment in the .properties file and will be ignored when parsed in java code. Trailing and leading spaces will also be trimmed in keys and values, just to name a few.
Also, this breaks round-tripping, i.e, an exported key will change when reimported since the unescaped special characters can result in unpredictable behavior during parsing.

I would be interested to know the motivation for this change based on the customer request in the linked issue. Any information on the customer's use case could help us advise a way forward.

Thanks!

@zhoxing-ms
Copy link
Copy Markdown
Contributor

@SaurabhSharma-MSFT Could you please take a look at the comment from albertofori ?

@SaurabhSharma-MSFT
Copy link
Copy Markdown
Member Author

Hi @albertofori ,
Please find below the details on customers's use case:

There are a couple of important counter-points here I want to make to the product team:

First, I think you are applying the escape rules for the key to the corresponding value as well. This is not what the document you referred to on Java Properties.load states:

Java Properties.load method

The key contains all of the characters in the line starting with the first non-white space character and up to, but not including, the first unescaped '=', ':', or white space character other than a line terminator. All of these key termination characters may be included in the key by escaping them with a preceding backslash character;

The following example in Java should demonstrate this more clearly using a simple URL as an example:

config.properties

myurl=https://127.0.0.1:8080
my:url=https://127.0.0.1:8080
my:url=https://127.0.0.1:8080
appConfigValue=!#:\
App.class

public class App {
public static void main(String[] args) {
try (InputStream input = App.class.getClassLoader().getResourceAsStream("config.properties")) {
Properties prop = new Properties();
prop.load(input);
System.out.println(prop.getProperty("myurl"));
System.out.println(prop.getProperty("my"));
System.out.println(prop.getProperty("my:url"));
System.out.println(prop.getProperty("appConfigValue"));

    } catch (IOException ex) {
        ex.printStackTrace();
    }
}

}
Output:

https://127.0.0.1:8080
url=https://127.0.0.1:8080
https://127.0.0.1:8080
!#:
However, if I have a simple URL value stored in Azure App Config of https://127.0.0.1:8080 and try to export it from the command line as a .properties file, the value returned contains escaped colons.

https://127.0.0.1:8080
Technically, this might still be acceptable in Java but remember that other languages use .properties for configuration. In my particular use case, I am using az appconfig kv export in a CI/CD pipeline in ADO to export a subset of configuration values and using the exported properties file to create a Kubernetes configmap object using. When importing in this manner, the escaped values are treated as literals in the K8s configmap which will break the service unless I introduce a rather ugly sed command to parse the values.

kubectl create configmap my-config --from-file=config.properties
Lastly, its important to point out that the export behavior of the az cli is also inconsistent with the behavior of the Import/Export feature within your own Azure App Configuration UI portal. If you open the Azure portal and add a configuration value of https://127.0.0.1:8080, in your app config instance, then export it to a properties file by navigating to "Import/Export" under "Operations" and select "Properties as the file type, the value will be exported without escaped characters. However, if you do the same thing with the az cli, it will export the value as https://127.0.0.1:8080

That's a big inconsistency in my opinion and would expect the behavior of both Azure export operations between the Portal UI and the CLI to be identical.

Please let me know if you have any other questions.

Thanks
Saurabh

@SaurabhSharma-MSFT
Copy link
Copy Markdown
Member Author

Hi @albertofori, Can you please look into the details shared above by the customer.

Comment thread src/azure-cli/azure/cli/command_modules/appconfig/_kv_helpers.py Outdated
Co-authored-by: Albert Ofori <102393878+albertofori@users.noreply.github.com>
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 18, 2023

Please fix CI issues.

@albertofori
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 25700 in repo Azure/azure-cli

@albertofori
Copy link
Copy Markdown
Contributor

@zhoxing-ms @yanzhudd , could you please help re-run the CI tests for this PR? We would want to proceed to merge this or fix any issues that currently exist.

@yanzhudd
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Sep 20, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@albertofori
Copy link
Copy Markdown
Contributor

@zhoxing-ms @yanzhudd Could you please help merge in this fix as it has been approved?

Thanks a lot!

@yanzhudd
Copy link
Copy Markdown
Contributor

yanzhudd commented Nov 6, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@albertofori
Copy link
Copy Markdown
Contributor

Issue fixed in this PR. Closing this out now.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[App Config] az appconfig kv export: escaped characters exporting to properties format

6 participants