Skip to content

fix(custom-domain-action): ignore A-record deletion if not found#4409

Merged
mergify[bot] merged 3 commits intoaws:mainlinefrom
efekarakus:fix/issues-4401-ignore-err
Jan 25, 2023
Merged

fix(custom-domain-action): ignore A-record deletion if not found#4409
mergify[bot] merged 3 commits intoaws:mainlinefrom
efekarakus:fix/issues-4401-ignore-err

Conversation

@efekarakus
Copy link
Copy Markdown
Contributor

Fixes #4401

If the record fails to insert, then the custom resource fails. On failure, it tries to delete the record that was not inserted. Hence the Delete workflow fails resulting in the custom resource to fail to delete.

This change ensures that we ignore the error if the A-record that we're trying to delete doesn't exist in the first place.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

Fixes aws#4401

If the record fails to insert, then the custom resource fails.
On failure, it tries to delete the record that was *not* inserted.
Hence the Delete workflow fails resulting in the custom resource to fail
to delete.

This change ensures that we ignore the error if the A-record that we're
trying to delete doesn't exist in the first place.
@efekarakus efekarakus requested a review from a team as a code owner January 24, 2023 23:07
@efekarakus efekarakus requested review from KollaAdithya and removed request for a team January 24, 2023 23:07
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 24, 2023

🍕 Here are the new binary sizes!

Name New size (kiB) size (kiB) Delta (%)
macOS (amd) 48556 48440 +0.24
macOS (arm) 49216 49108 +0.22
linux (amd) 42716 42608 +0.25
linux (arm) 42048 41924 +0.30
windows (amd) 39468 39368 +0.25

Comment thread cf-custom-resources/lib/custom-domain.js
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 24, 2023

Codecov Report

Merging #4409 (7c9bbc2) into mainline (a89fadc) will decrease coverage by 0.01%.
The diff coverage is 88.88%.

@@             Coverage Diff              @@
##           mainline    #4409      +/-   ##
============================================
- Coverage     70.06%   70.06%   -0.01%     
============================================
  Files           262      262              
  Lines         37324    37330       +6     
  Branches        266      268       +2     
============================================
+ Hits          26152    26155       +3     
- Misses         9915     9919       +4     
+ Partials       1257     1256       -1     
Impacted Files Coverage Δ
cf-custom-resources/lib/custom-domain.js 89.74% <88.88%> (-0.26%) ⬇️
...ternal/pkg/deploy/cloudformation/cloudformation.go 72.47% <0.00%> (-0.55%) ⬇️
internal/pkg/deploy/cloudformation/stack/env.go 79.67% <0.00%> (-0.06%) ⬇️

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

Comment thread cf-custom-resources/lib/custom-domain.js
Comment thread cf-custom-resources/test/custom-domain-test.js
@paragbhingre paragbhingre added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jan 25, 2023
Copy link
Copy Markdown
Contributor

@paragbhingre paragbhingre left a comment

Choose a reason for hiding this comment

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

Looks good to me. Feel free to remove the label if there is nothing to change.

@efekarakus efekarakus removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Jan 25, 2023
@mergify mergify Bot merged commit c01b36b into aws:mainline Jan 25, 2023
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.

HTTP Alias Issue left HTTPSCert and CustomDomainAction stuck in UPDATE_FAILED

5 participants