Add support for Subscription creation in ASO #2446
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthchr
requested review from
davefellows,
theunrepentantgeek,
Porges,
babbageclunk and
super-harsh
as code owners
super-harsh
reviewed
Aug 12, 2022
super-harsh
reviewed
Aug 12, 2022
super-harsh
reviewed
Aug 12, 2022
| case ResourceHierarchyRootTenant: | ||
| // The only resource we actually care about for figuring out resource types is the | ||
| // most derived resource | ||
| res := h[len(h)-1] |
There was a problem hiding this comment.
could this run into indexOutOfBounds if len==0?
There was a problem hiding this comment.
ok, makes sense .. it panics in rootKind method if len==0
There was a problem hiding this comment.
It could, yes. There are other places where we explicitly panic on len==0, but basically a ResourceHierarchy is expected to be at least length = 1. Moreover, these are created by functions we control and they always have at least the resource being deployed in them, so there's no way in practice to end up with 1 that's length = 0.
Good question though.
super-harsh
reviewed
Aug 12, 2022
Includes using various extensions to make this resource behave a bit more like expected
matthchr
force-pushed
the
feature/subscriptions
branch
from
7fbf05a to
c0f83fd
Compare
|
/ok-to-test sha=c0f83fd |
matthchr
force-pushed
the
feature/subscriptions
branch
from
c0f83fd to
daa372b
Compare
|
/ok-to-test sha=daa372b |
theunrepentantgeek
approved these changes
Aug 14, 2022
|
/ok-to-test sha=bfc2610 |
|
/ok-to-test sha=d5f6faa |
|
/ok-to-test sha=32bfbfb |
1 similar comment
|
/ok-to-test sha=32bfbfb |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This resource is a bit wonky/tricky and as such required a few hooks to customize its reconciliation behavior from what the default is.
The underlying API is that creating a subscription requires creating an Alias. That Alias is not the subscription itself, just a mechanism to produce a new one. Unfortunately the Alias cannot have RoleAssignments or other things put on it once it's created, that must be done on the subscription itself.
In order to manage this resource in ASO we combine the two concepts into a single one:
subscription.Alias. Deleting the Alias causes the subscription to be cancelled. A subscription cannot be cancelled if it has resources (delete will fail with an error). A subscription that is cancelled will be deleted 90 days later (or can be manually deleted after 3 days).Blocked until
How does this PR make you feel:
If applicable: