feat: Refactor set-namespace #785
Merged
Conversation
This file contains hidden or 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
yuwenma
changed the title
yuwenma
force-pushed
the
set-namespace-v2
branch
2 times, most recently
from
6bd45e7 to
bd2a258
Compare
justinsb
reviewed
Mar 24, 2022
yuwenma
force-pushed
the
set-namespace-v2
branch
3 times, most recently
from
b5723c3 to
7aeb38a
Compare
yuwenma
changed the title
yuwenma
force-pushed
the
set-namespace-v2
branch
from
7aeb38a to
596d5f7
Compare
justinsb
approved these changes
Mar 29, 2022
|
I like this callback approach a lot 👍 |
yuwenma
force-pushed
the
set-namespace-v2
branch
5 times, most recently
from
4db2c75 to
200f250
Compare
- Define visitNamespace with callback function as params to replace multiple individual object iterations. - Define Namespace struct to provide callback functions for different iterations
yuwenma
force-pushed
the
set-namespace-v2
branch
from
200f250 to
fcba72c
Compare
Thank you so much for the suggestions! |
yuwenma
force-pushed
the
set-namespace-v2
branch
from
fcba72c to
f9c2dc8
Compare
merge after
yuwenma
force-pushed
the
set-namespace-v2
branch
from
f9c2dc8 to
d9db966
Compare
* updated behavior and tests for ensure-name-substring * adjusted fix-simple test to not specify include-meta-resources * kubeval-simple: skip Kptfile validation * adjusted starlark tests/examples to handle Kptfile and StarlarkRun files * adjusted search-replace tests to match kpt behavior to include meta resources by default * adjusted create-setters-simple test to match kpt behavior * adjusted set-project-id to match the kpt fn behavior * adjusted generate-kpt-pkg-docs-simple test to match kpt behavior * adjusted set-annotations tests to match new kpt fn behavior * adjusted set-labels test to match new kpt fn behavior * adjusted the golden file for generate-folders function * using kpt@main
yuwenma
force-pushed
the
set-namespace-v2
branch
from
8148ca3 to
38ddcdb
Compare
mengqiy
reviewed
Apr 6, 2022
functions/go/set-namespace/README.md
Outdated
| namespace: irrelevant # skip since namespace does not match "example". | ||
| ``` | ||
|
|
||
| ##### Selector Mode |
There was a problem hiding this comment.
We should have at least one example for each mode.
There was a problem hiding this comment.
I'll do that in a follow up PR.
mengqiy
approved these changes
Apr 6, 2022
yuwenma
added a commit
that referenced
this pull request
May 4, 2022
Co-authored-by: Sunil Arora <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
New logic
set-namespacesupports three modesnamespaceSelectorparameter in the functionConfig. If given, thenamespaceSelectorvalue is the "oldNs"namespaceobject in the resourcelist.items. If found (andnamespaceSelectoris not given), use the namespacemetadata.namevalue as "oldNs"namespaceSelectornornamespaceobject are found, we require all namespace scoped resources have the same namespace value, which is the "oldNs".set-namespaceonly replace the namespace fields which match the "oldNs" to the new Namespace.config.kubernetes.io/local-config"ClusterRoleBinding" and "RoleBinding" changes
RoleBindingsubjectsnamespace check. BecauseRoleBindingis namespace-scoped so it only works for resources under the same namespace. Itselfmetadata.namespaceshould be updated.ClusterRoleBindingcheck to not check subjects "ServiceAccount" kind. For clusterrolebinding, only ServiceAccount may be namespace scoped.New Code Structure
Define
VisitNamespacewith callback function as params to iterationsresourcelist.items.Define
Namespacestruct to provide callback functions for different iterations, includingFindAllNamespacesto check duplication,UpdateNamespaceto update namespace value via string pointer,GetDependsOnMapto build a key value pairs by resourcesdepends-onannotation, andUpdateAnnotationto update the namespace segment in thedepends-onannotation.Call the new function in go-sdk
IsGVKMatchfor duck typing check.Update the KRM resources' namespace value via string pointer.
Rewrite the README