Localize resources #4912

annasong20 · 2022-12-07T05:33:18Z

This PR localizes entries under the resources kustomization field.

annasong20 · 2022-12-07T05:33:44Z

api/internal/localizer/errors.go

natasha41575 · 2022-12-08T17:58:48Z

api/internal/localizer/localizer_test.go

@@ -753,3 +753,58 @@ nameSuffix: -test
 	addFiles(t, fSysExpected, "/localized", kustAndComponents)
 	checkFSys(t, fSysExpected, fSys)
 }
+
+func TestLocalizeResources(t *testing.T) {


Could we add a tests here with a remote resource file, and a remote resource directory?

We can't add that test yet because the code that calculates the path to save a remote root, #4909, hasn't been merged yet.

Also, since these are integration tests, I planned to add them in a separate PR in krusty. I can add them in krusty in this PR once #4909 is merged?

I thought that the localizeFile and localizeDir functions are supposed to be converting the remote URLs to local filepaths, so IMO these tests should cover that behavior. Can we get #4909 in and then add the tests here?

I'm also OK to merge this before #4909, if we add these tests in #4909. Let me know which you prefer.

I thought that the localizeFile and localizeDir functions are supposed to be converting the remote URLs to local filepaths, so IMO these tests should cover that behavior. Can we get #4909 in and then add the tests here?

Sure, sg.

yuwenma

LGTM on my side. Can you resolve @natasha41575 comment about the unittest before merging?

annasong20 · 2022-12-13T17:25:08Z

/uncc @koba1t

k8s-ci-robot · 2022-12-13T17:56:00Z

@annasong20: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

annasong20 · 2022-12-13T17:56:17Z

/label tide/merge-method-squash

yuwenma · 2022-12-15T01:12:00Z

/lgtm

natasha41575

Tests LGTM. I just have a small comment about organization.

natasha41575 · 2023-01-04T17:14:24Z

api/krusty/localizer/runner_test.go

 )

-func prepareFs(t *testing.T, files map[string]string) (memoryFs filesys.FileSystem, actualFs filesys.FileSystem, testDir filesys.ConfirmedDir) {
+func prepareFs(t *testing.T, dirs []string, files map[string]string) (


Since we have a lot of helper functions now, can we move these helpers to their own file api/krusty/localizer/testutils.go? I think it will help keep this directory organized.

I moved all the helper functions to the top of the file for now. I'm all for improving organization. The problem with moving the helper functions to testutils.go is this file can't exist in package localizer_test because the file name doesn't end in _test.go. We don't want the test helper functions to be publicly exposed in the localizer package either.

If moving the helper functions to the top of the file is not clean enough, I came up with a few other options:

Create a loctest package under api/testutils and move the helper functions to a file there called helper.go.

Create a testutils package under api/krusty/localizer/internal and move the helper functions to a file there called helper.go.

Move the helper functions to a new utils_test.go file under api/krusty/localizer.

natasha41575

/lgtm

k8s-ci-robot · 2023-01-05T16:46:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annasong20, natasha41575, yuwenma

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:

~~OWNERS~~ [natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* Localize resources * Improve readability * Add integration tests * Group test helper functions

* initial changes to rename OrgRepo to RepoPath * changes to rename Path to KustRootPath * addressed review comments * addressed review comments * docs: Add documentation for namespace transformer Add a short description of the namespace transformer and example usage to examples/transformerconfigs/README.md. References: #629 Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com> * Localize patchesJson6902, patchesStrategicMerge, replacements (#4904) * Localize patchesJson6902, patchesStrategicMerge, replacements * Address code review feedback * Improve readability * Remove deprecation warning check * Load legacy kustomization fields for `localize` (#4918) * Load legacy kustomization * Expose loadKustFile in kusttarget * remove FixKustomizationPreUnmarshalling * remove deprecated cfg and fn commands (#4930) * remove deprecated cfg and fn commands * fix lint error * run gofmt * Localize PatchTransformer, PatchJson6902Transformer (#4920) * Localize patches, patchesJson6902 custom transformers * Improve readability * Localize fields: openapi, configurations, crds (#4907) * Localize openapi, configurations, crds * Add integration test * Move krusty test * Address code review feedback * Implement locRootPath (#4909) * Implement locRootPath, and include userinfo, port in locFilePath * Strip userinfo, port * Improve readability * Localize legacy fields * Localize resources (#4912) * Localize resources * Improve readability * Add integration tests * Group test helper functions * Remove Functionality that Pulls Env Variables from Empty Keys * Update api/kv/kv.go Co-authored-by: Katrina Verey <kn.verey@gmail.com> * refactor Unmarshal Kustomization struct code * improve error messages * Run go mod tidy on all modules before update * Update sigs.k8s.io/yaml to 1.3.0 * fixed test failure because of latest commits Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com> Co-authored-by: Lars Kellogg-Stedman <lars@oddbit.com> Co-authored-by: Anna Song <annasong@google.com> Co-authored-by: yugo kobayashi <kobdotsh@gmail.com> Co-authored-by: Natasha Sarkar <natashasarkar@google.com> Co-authored-by: Cailyn Edwards <cailyn.edwards@shopify.com> Co-authored-by: Cailyn <cailyn.s.e@gmail.com> Co-authored-by: Katrina Verey <kn.verey@gmail.com> Co-authored-by: Katrina Verey <katrina.verey@shopify.com>

* Localize resources * Improve readability * Add integration tests * Group test helper functions

* initial changes to rename OrgRepo to RepoPath * changes to rename Path to KustRootPath * addressed review comments * addressed review comments * docs: Add documentation for namespace transformer Add a short description of the namespace transformer and example usage to examples/transformerconfigs/README.md. References: kubernetes-sigs#629 Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com> * Localize patchesJson6902, patchesStrategicMerge, replacements (kubernetes-sigs#4904) * Localize patchesJson6902, patchesStrategicMerge, replacements * Address code review feedback * Improve readability * Remove deprecation warning check * Load legacy kustomization fields for `localize` (kubernetes-sigs#4918) * Load legacy kustomization * Expose loadKustFile in kusttarget * remove FixKustomizationPreUnmarshalling * remove deprecated cfg and fn commands (kubernetes-sigs#4930) * remove deprecated cfg and fn commands * fix lint error * run gofmt * Localize PatchTransformer, PatchJson6902Transformer (kubernetes-sigs#4920) * Localize patches, patchesJson6902 custom transformers * Improve readability * Localize fields: openapi, configurations, crds (kubernetes-sigs#4907) * Localize openapi, configurations, crds * Add integration test * Move krusty test * Address code review feedback * Implement locRootPath (kubernetes-sigs#4909) * Implement locRootPath, and include userinfo, port in locFilePath * Strip userinfo, port * Improve readability * Localize legacy fields * Localize resources (kubernetes-sigs#4912) * Localize resources * Improve readability * Add integration tests * Group test helper functions * Remove Functionality that Pulls Env Variables from Empty Keys * Update api/kv/kv.go Co-authored-by: Katrina Verey <kn.verey@gmail.com> * refactor Unmarshal Kustomization struct code * improve error messages * Run go mod tidy on all modules before update * Update sigs.k8s.io/yaml to 1.3.0 * fixed test failure because of latest commits Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com> Co-authored-by: Lars Kellogg-Stedman <lars@oddbit.com> Co-authored-by: Anna Song <annasong@google.com> Co-authored-by: yugo kobayashi <kobdotsh@gmail.com> Co-authored-by: Natasha Sarkar <natashasarkar@google.com> Co-authored-by: Cailyn Edwards <cailyn.edwards@shopify.com> Co-authored-by: Cailyn <cailyn.s.e@gmail.com> Co-authored-by: Katrina Verey <kn.verey@gmail.com> Co-authored-by: Katrina Verey <katrina.verey@shopify.com>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 7, 2022

k8s-ci-robot requested review from koba1t and natasha41575 December 7, 2022 05:33

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 7, 2022

k8s-ci-robot requested a review from yuwenma December 7, 2022 05:33

ybettan mentioned this pull request Dec 7, 2022

Adding branch_protection to the kernel-module-management project. kubernetes/test-infra#28198

Merged

yuwenma suggested changes Dec 7, 2022

View reviewed changes

api/internal/localizer/errors.go Outdated Show resolved Hide resolved

api/internal/localizer/errors.go Outdated Show resolved Hide resolved

natasha41575 reviewed Dec 8, 2022

View reviewed changes

yuwenma approved these changes Dec 9, 2022

View reviewed changes

annasong20 force-pushed the localize-resources branch from d62f9cf to 59470ac Compare December 13, 2022 17:55

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 13, 2022

k8s-ci-robot assigned yuwenma Dec 15, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 15, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 23, 2022

annasong20 force-pushed the localize-resources branch from 59470ac to b574f8a Compare January 3, 2023 19:06

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 3, 2023

natasha41575 reviewed Jan 4, 2023

View reviewed changes

annasong20 added 2 commits January 4, 2023 12:46

Localize resources

bf42302

Improve readability

3576a31

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 4, 2023

annasong20 added 2 commits January 4, 2023 12:49

Add integration tests

599d24c

Group test helper functions

1f14895

annasong20 force-pushed the localize-resources branch from b574f8a to 1f14895 Compare January 5, 2023 00:34

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 5, 2023

annasong20 mentioned this pull request Jan 5, 2023

Localize patch, ReplacementTransformer #4943

Merged

natasha41575 approved these changes Jan 5, 2023

View reviewed changes

k8s-ci-robot assigned natasha41575 Jan 5, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 5, 2023

k8s-ci-robot merged commit 06da3b9 into kubernetes-sigs:master Jan 5, 2023

annasong20 deleted the localize-resources branch January 5, 2023 17:19

kishorerj pushed a commit to kishorerj/kustomize that referenced this pull request Jan 8, 2023

Localize resources (kubernetes-sigs#4912)

6736c37

* Localize resources * Improve readability * Add integration tests * Group test helper functions

cailyn-codes pushed a commit to cailyn-codes/kustomize that referenced this pull request Jan 12, 2023

Localize resources (kubernetes-sigs#4912)

cc20673

* Localize resources * Improve readability * Add integration tests * Group test helper functions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Localize resources #4912

Localize resources #4912

annasong20 commented Dec 7, 2022

annasong20 commented Dec 7, 2022

natasha41575 Dec 8, 2022

annasong20 Dec 8, 2022

natasha41575 Dec 8, 2022

natasha41575 Dec 9, 2022 •

edited

Loading

annasong20 Dec 9, 2022

yuwenma left a comment

annasong20 commented Dec 13, 2022

k8s-ci-robot commented Dec 13, 2022

annasong20 commented Dec 13, 2022

yuwenma commented Dec 15, 2022

natasha41575 left a comment

natasha41575 Jan 4, 2023

annasong20 Jan 5, 2023

natasha41575 left a comment

k8s-ci-robot commented Jan 5, 2023

Localize resources #4912

Localize resources #4912

Conversation

annasong20 commented Dec 7, 2022

annasong20 commented Dec 7, 2022

natasha41575 Dec 8, 2022

Choose a reason for hiding this comment

annasong20 Dec 8, 2022

Choose a reason for hiding this comment

natasha41575 Dec 8, 2022

Choose a reason for hiding this comment

natasha41575 Dec 9, 2022 • edited Loading

Choose a reason for hiding this comment

annasong20 Dec 9, 2022

Choose a reason for hiding this comment

yuwenma left a comment

Choose a reason for hiding this comment

annasong20 commented Dec 13, 2022

k8s-ci-robot commented Dec 13, 2022

annasong20 commented Dec 13, 2022

yuwenma commented Dec 15, 2022

natasha41575 left a comment

Choose a reason for hiding this comment

natasha41575 Jan 4, 2023

Choose a reason for hiding this comment

annasong20 Jan 5, 2023

Choose a reason for hiding this comment

natasha41575 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 5, 2023

natasha41575 Dec 9, 2022 •

edited

Loading