Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[AppConfig] az appconfig kv delete/set/set-keyvault: Add key validations for null or empty space key parameter #26928

Merged
merged 2 commits into from
Jul 19, 2023
Merged

[AppConfig] az appconfig kv delete/set/set-keyvault: Add key validations for null or empty space key parameter #26928

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1148df7
Adding key validations for null or empty space keys.
albertofori Jul 18, 2023
6f92cbc
Update to use validate key method only.
albertofori Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/azure-cli/azure/cli/command_modules/appconfig/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
validate_key, validate_feature, validate_feature_key,
validate_identity, validate_auth_mode,
validate_resolve_keyvault, validate_export_profile, validate_import_profile,
validate_strict_import, validate_export_as_reference, validate_snapshot_filters)
validate_strict_import, validate_export_as_reference, validate_snapshot_filters,
validate_non_empty_key)


def load_arguments(self, _):
Expand Down Expand Up @@ -250,7 +251,7 @@ def load_arguments(self, _):
c.argument('secret_identifier', validator=validate_secret_identifier, help="ID of the Key Vault object. Can be found using 'az keyvault {collection} show' command, where collection is key, secret or certificate. To set reference to the latest version of your secret, remove version information from secret identifier.")

with self.argument_context('appconfig kv delete') as c:
c.argument('key', help='Support star sign as filters, for instance * means all key and abc* means keys with abc as prefix.')
c.argument('key', validator=validate_non_empty_key, help='Support star sign as filters, for instance * means all key and abc* means keys with abc as prefix.')
Copy link
Member

Choose a reason for hiding this comment

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

Why not use validate_key directly? The character restrictions apply for delete command too.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. The reason for separating the two was that the delete parameter could also be a filter. But specifying any of the restricted characters without any wildcard should return nothing from the backend anyway since no kv could have been created with such a key. As you rightly pointed out, it is even rejected by the backend.

This has been updated, thanks!

c.argument('label', help="If no label specified, delete entry with null label. Support star sign as filters, for instance * means all label and abc* means labels with abc as prefix.")

with self.argument_context('appconfig kv show') as c:
Expand Down
14 changes: 9 additions & 5 deletions src/azure-cli/azure/cli/command_modules/appconfig/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,15 @@ def validate_secret_identifier(namespace):


def validate_key(namespace):
if namespace.key:
input_key = str(namespace.key).lower()
if input_key == '.' or input_key == '..' or '%' in input_key:
raise InvalidArgumentValueError("Key is invalid. Key cannot be a '.' or '..', or contain the '%' character.")
else:
validate_non_empty_key(namespace)

input_key = str(namespace.key).lower()
if input_key == '.' or input_key == '..' or '%' in input_key:
raise InvalidArgumentValueError("Key is invalid. Key cannot be a '.' or '..', or contain the '%' character.")


def validate_non_empty_key(namespace):
if not namespace.key or str(namespace.key).isspace():
raise RequiredArgumentMissingError("Key cannot be empty.")


Expand Down
3,002 changes: 1,619 additions & 1,383 deletions ...ure-cli/azure/cli/command_modules/appconfig/tests/latest/recordings/test_azconfig_kv.yaml
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/test_appconfig_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from azure.cli.testsdk.scenario_tests import AllowLargeResponse, RecordingProcessor
from azure.cli.testsdk.scenario_tests.utilities import is_json_payload
from azure.core.exceptions import ResourceNotFoundError
from azure.cli.core.azclierror import ResourceNotFoundError as CliResourceNotFoundError
from azure.cli.core.azclierror import ResourceNotFoundError as CliResourceNotFoundError, RequiredArgumentMissingError
from azure.cli.core.util import shell_safe_json_parse

TEST_DIR = os.path.abspath(os.path.join(os.path.abspath(__file__), '..'))
Expand Down Expand Up @@ -411,6 +411,10 @@ def test_azconfig_kv(self, resource_group, location):
'appconfig revision list -n {config_store_name} --key {key} --label *').get_output_in_json()
assert len(revisions) == 3

# Confirm that delete action errors out for empty or whitespace key
with self.assertRaisesRegex(RequiredArgumentMissingError, "Key cannot be empty."):
self.cmd('appconfig kv delete -n {config_store_name} --key " " -y')

# IN CLI, since we support delete by key/label filters, return is a list of deleted items
deleted = self.cmd('appconfig kv delete -n {config_store_name} --key {key} --label {label} -y',
checks=[self.check('[0].key', entry_key),
Expand Down