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.

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

APIM: Delete subscriptions when deleting products #3467

Merged
merged 7 commits into from
Oct 27, 2023

Conversation

ross-p-smith
Copy link
Contributor

@ross-p-smith ross-p-smith commented Oct 25, 2023

This pull request fixes two APIM issues

Goal Seeker behaviour

My previous work on APIM left a setting of Recover: true when creating the apim instance. This will cause an error if you were to delete the recording and not have an apim instance in a deleted state.

It would be better not to have this setting defined and allow it to error if there was a deleted APIM in your resource group - which is a smaller probability!

This would eventually be handled by having a goal seeking APIM creation #3433.

Test deletion of products

When we create an APIM Product, it creates a Subscription for that Product. This causes problems when you delete the Product; however there is a flag to pass deleteSubscriptions to true which will fix this. Override the default delete behaviour and use the go sdk to carry out the deletion. This will close #3408

How does this PR make you feel:
gif

If applicable:

  • this PR contains documentation
  • this PR contains tests

@ross-p-smith ross-p-smith changed the title APIM: Should not try and recover by default APIM: Delete subscriptions when deleting products Oct 26, 2023
Copy link
Member

@theunrepentantgeek theunrepentantgeek left a comment

Choose a reason for hiding this comment

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

Only needs a couple of minor updates, then will be ready to merge.

@theunrepentantgeek
Copy link
Member

/ok-to-test sha=83b50e8

@theunrepentantgeek
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Bevan Arps <bevan.arps@outlook.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2023

Codecov Report

Merging #3467 (f8daa8b) into main (db857cf) will decrease coverage by 0.02%.
The diff coverage is 62.96%.

@@            Coverage Diff             @@
##             main    #3467      +/-   ##
==========================================
- Coverage   54.32%   54.31%   -0.02%     
==========================================
  Files        1566     1567       +1     
  Lines      655434   655461      +27     
==========================================
- Hits       356073   356015      -58     
- Misses     241588   241641      +53     
- Partials    57773    57805      +32     
Files Coverage Δ
...apimanagement/customizations/product_extensions.go 62.96% <62.96%> (ø)

... and 49 files with indirect coverage changes

@theunrepentantgeek
Copy link
Member

/ok-to-test sha=f8daa8b

@theunrepentantgeek
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theunrepentantgeek theunrepentantgeek added this pull request to the merge queue Oct 27, 2023
Merged via the queue into Azure:main with commit b2428ee Oct 27, 2023
8 checks passed
@ross-p-smith ross-p-smith deleted the ross/recover_apim branch October 30, 2023 13:36
@theunrepentantgeek theunrepentantgeek added this to the v2.4.0 milestone Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

APIM: Product should delete any Subscriptions it owns when it deletes
3 participants