Force creation of generic items into AppWrapper deployment namespace #665
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
astefanutti
reviewed
Oct 19, 2023
There was a problem hiding this comment.
This looks great!
For this PR: #652 (I have to update it)
Should I still keep the resourceName labels but left unused as discussed?
Ok. I added the same fix on resource deletion. |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: astefanutti The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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 PR makes sure that generic items are created in the same namespace as the AppWrapper. If the metadata of the generic item explicitly specifies a namespace that is different from the AppWrapper deployment namespace, an error message is generated to report the discrepancy and the resource creation fails. If the specified namespace is equal to the AppWrapper namespace or if no explicit namespace is specified then the creation can process in the AppWrapper namespace.
This PR does not affect the creation of cluster scoped resources, which was already prohibited.
This PR modifies MCAD's behavior when resources do not specify a namespace. They used to be deployed in the
defaultnamespace irrespective of the AppWrapper namespace. Now they will be deployed in the AppWrapper namespace instead. Alternatively, we could check the namespace without changing the existing behavior. In the long term though, if the deployment namespace can only be the appwrapper namespace, it does not make much sense to require the namespace in the resource definition, i.e., interpret the absence of a namespace as thedefaultnamespace.