Skip to content

fix(add-secret): SecretString can be None#5672

Merged
bblommers merged 2 commits intogetmoto:masterfrom
jfagoagas:fix-secretsmanager-rotate-secret
Nov 18, 2022
Merged

fix(add-secret): SecretString can be None#5672
bblommers merged 2 commits intogetmoto:masterfrom
jfagoagas:fix-secretsmanager-rotate-secret

Conversation

@jfagoagas
Copy link
Copy Markdown
Contributor

@jfagoagas jfagoagas commented Nov 15, 2022 •
edited
Loading

Description

As per Boto3 documentation the SecretString create_secret API parameter is not required.

So if you try to enable rotation in a previously created secret without a SecretString, the moto rotate_secret fails with a KeyError due to the inexistence of old_secret_version["secret_string"]

@mock_secretsmanager
    def test_create_and_rotate_secret(self):
        secretsmanager_client = client("secretsmanager", region_name="eu-west-1")
        # Create Secret
        resp = secretsmanager_client.create_secret(
            Name="test-secret"
        )
        secret_arn = resp["ARN"]
        secret_name = resp["Name"]
        # Enable Rotation
        secretsmanager_client.rotate_secret( # <-- this call fails with a KeyError
            SecretId=secret_arn,
        )

@jfagoagas jfagoagas changed the title fix(add-secret): secret_string can be None fix(add-secret): SecretString can be None Nov 15, 2022
bblommers
bblommers requested changes Nov 16, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Hi @jfagoagas, thanks for the PR! Would you mind adding a unit test for this?

@jfagoagas jfagoagas force-pushed the fix-secretsmanager-rotate-secret branch from af9f93b to 1183940 Compare November 18, 2022 08:02
@jfagoagas
Copy link
Copy Markdown
Contributor Author

jfagoagas commented Nov 18, 2022

Hi @jfagoagas, thanks for the PR! Would you mind adding a unit test for this?

Sure, done!

@jfagoagas jfagoagas force-pushed the fix-secretsmanager-rotate-secret branch from 1183940 to cf20a78 Compare November 18, 2022 08:19
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 18, 2022

Codecov Report

Merging #5672 (cf20a78) into master (df64b7b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5672   +/-   ##
=======================================
  Coverage   96.20%   96.20%           
=======================================
  Files         743      743           
  Lines       72740    72808   +68     
=======================================
+ Hits        69977    70043   +66     
- Misses       2763     2765    +2
Flag Coverage Δ
servertests 36.87% <100.00%> (-0.01%) ⬇️
unittests 96.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
moto/secretsmanager/models.py 98.61% <100.00%> (+<0.01%) ⬆️
moto/dynamodb/parsing/expressions.py 96.14% <0.00%> (-0.18%) ⬇️
moto/dynamodb/models/__init__.py 93.58% <0.00%> (-0.09%) ⬇️
moto/ec2/utils.py 93.63% <0.00%> (ø)
moto/ecs/models.py 93.09% <0.00%> (ø)
moto/elb/models.py 91.59% <0.00%> (ø)
moto/emr/models.py 97.50% <0.00%> (ø)
moto/elbv2/utils.py 100.00% <0.00%> (ø)
moto/elbv2/models.py 92.55% <0.00%> (ø)
moto/ecr/responses.py 95.56% <0.00%> (ø)
... and 39 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bblommers
bblommers approved these changes Nov 18, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

LGTM!

@bblommers bblommers merged commit dfd45d8 into getmoto:master Nov 18, 2022
@bblommers bblommers added this to the 4.0.10 milestone Nov 18, 2022
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 18, 2022

This is now part of moto >= 4.0.10.dev40

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

Reviewers

@bblommers bblommers bblommers approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0.10

Development

Successfully merging this pull request may close these issues.

3 participants

@jfagoagas @codecov-commenter @bblommers