Add --no-verify flag to edit add resource command #5333

hailkomputer · 2023-09-18T18:30:14Z

This PR addresses the issue #3276

Users want to be able to run kustomize edit add resource <remote-resource> in cases the remote is not reachable and can not be validated.

--skip-validation flag will be passed to the underlying GlobPatternsWithLoader function which is refactored here.
Refactored behavior of GlobPatternsWithLoader function is as follows:

It accepts a slice of glob strings and returns the set of matching file paths. If validation is skipped, then it will return the patterns as provided. Otherwise, It will try to load the files from the filesystem. If files are not found in the filesystem, it will try to load from remote. It returns an error if validation is not skipped and there are no matching files or it can't load from remote.

Other instances of the function reference is also changed with the newly introduces input parameter.

k8s-ci-robot · 2023-09-18T18:55:27Z

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.

natasha41575 · 2023-09-21T15:07:42Z

/cc @mehabhalodiya

kustomize/commands/internal/util/util.go

kustomize/commands/internal/util/util_test.go

hailkomputer · 2023-09-27T10:42:51Z

@stormqueen1990 Thank you for the suggestions, I have addressed them in the latest commit.

stormqueen1990

/lgtm

charles-chenzz

/lgtm

natasha41575

LGTM - except for just one comment on the name of the flag.

natasha41575 · 2023-10-02T19:44:21Z

kustomize/commands/edit/add/addresource.go

@@ -35,6 +36,9 @@ func newCmdAddResource(fSys filesys.FileSystem) *cobra.Command {
 			return o.RunAddResource(fSys)
 		},
 	}
+	cmd.Flags().BoolVar(&o.skipValidation, "skip-validation", false,


For parity with the flag we are adding to kustomize localize (#5276), can we likewise name this flag --no-verify?

That would be better. I've renamed the flag.

natasha41575 · 2023-10-02T19:45:30Z

kustomize/commands/internal/util/util_test.go

+	resources, err = GlobPatternsWithLoader(fSys, ldr, []string{invalidURL}, true)
+	require.NoError(t, err, "unexpected load error")
+	require.Equal(t, 1, len(resources), "incorrect resources")
+	require.Equal(t, invalidURL, resources[0], "incorrect resources")


Thanks for fixing up these tests and adding one for this feature.

I think it might be nice to rewrite these tests using test structs and subtests, here is an example of this for the remove tests, so that it is easier to read and distinguish each test. You don't need to do this as part of this PR, but it would be a welcome change if you'd like to submit a follow-up PR to do so.

Yes, I was also thinking about that when I refactored the tests. Let me tackle that in a separate PR.

natasha41575 · 2023-10-06T17:57:21Z

/lgtm
/approve

Thanks!

k8s-ci-robot · 2023-10-06T17:57:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hailkomputer, natasha41575

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

Add skip-validation flag to edit add resource cmd

f3b34c4

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 18, 2023

k8s-ci-robot requested review from koba1t and yuwenma September 18, 2023 18:30

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 18, 2023

modified to fix linter issues

df0cd3c

k8s-ci-robot requested a review from mehabhalodiya September 21, 2023 15:07

natasha41575 added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 21, 2023

stormqueen1990 reviewed Sep 24, 2023

View reviewed changes

refactor tests to use require instead

039b05f

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 27, 2023

hailkomputer added 2 commits September 27, 2023 12:49

modify add resource test to accomodate changes

cb5b241

modify error message to original

0d854a5

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

stormqueen1990 reviewed Oct 1, 2023

View reviewed changes

k8s-ci-robot assigned stormqueen1990 Oct 1, 2023

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

Merge branch 'master' into add-skip-validation-flag-to-edit-add-resource

c3a4f3e

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 Oct 2, 2023

fix imports after rebase

5936a89

charles-chenzz reviewed Oct 2, 2023

View reviewed changes

k8s-ci-robot assigned charles-chenzz Oct 2, 2023

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

natasha41575 reviewed Oct 2, 2023

View reviewed changes

rename flag to --no-verify

7138423

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 3, 2023

hailkomputer requested a review from natasha41575 October 3, 2023 07:06

hailkomputer changed the title ~~Add skip-validation flag to edit add resource command~~ Add --no-verify flag to edit add resource command Oct 3, 2023

k8s-ci-robot assigned natasha41575 Oct 6, 2023

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

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

natasha41575 mentioned this pull request Oct 6, 2023

kustomize edit add resource remoteurl shouldn't require validation #3276

Closed

natasha41575 merged commit ace2d53 into kubernetes-sigs:master Oct 6, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --no-verify flag to edit add resource command #5333

Add --no-verify flag to edit add resource command #5333

hailkomputer commented Sep 18, 2023

k8s-ci-robot commented Sep 18, 2023

natasha41575 commented Sep 21, 2023

hailkomputer commented Sep 27, 2023

stormqueen1990 left a comment

charles-chenzz left a comment

natasha41575 left a comment •

edited

Loading

natasha41575 Oct 2, 2023 •

edited

Loading

hailkomputer Oct 3, 2023

natasha41575 Oct 2, 2023 •

edited

Loading

hailkomputer Oct 3, 2023

natasha41575 commented Oct 6, 2023

k8s-ci-robot commented Oct 6, 2023

Add --no-verify flag to edit add resource command #5333

Add --no-verify flag to edit add resource command #5333

Conversation

hailkomputer commented Sep 18, 2023

k8s-ci-robot commented Sep 18, 2023

natasha41575 commented Sep 21, 2023

hailkomputer commented Sep 27, 2023

stormqueen1990 left a comment

Choose a reason for hiding this comment

charles-chenzz left a comment

Choose a reason for hiding this comment

natasha41575 left a comment • edited Loading

Choose a reason for hiding this comment

natasha41575 Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

hailkomputer Oct 3, 2023

Choose a reason for hiding this comment

natasha41575 Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

hailkomputer Oct 3, 2023

Choose a reason for hiding this comment

natasha41575 commented Oct 6, 2023

k8s-ci-robot commented Oct 6, 2023

natasha41575 left a comment •

edited

Loading

natasha41575 Oct 2, 2023 •

edited

Loading

natasha41575 Oct 2, 2023 •

edited

Loading