Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add kustomize edit add generator command #4361

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

natasha41575
Copy link
Contributor

Fix #4343

Per request, create a command kustomize edit add generator. Just like the existing kustomize edit add transformer, but for generators. This code is almost identical to the code in addtransformer.go and addtransormer_test.go.

/cc @KnVerey
/cc @yuwenma

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 31, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 31, 2021
}

// Validate validates add generator command.
func (o *addGeneratorOptions) Validate(args []string) error {
Copy link
Contributor

@yuwenma yuwenma Jan 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to add validation to make sure

  • The existence of the path. Reason: if not checked, kustomize build will fail for the same reason anyway. it makes kustomize edit feel less reliable/stable since it edits user files without necessary validation checks even the same logic is used in kustomize build.
  • Format of the generator. Reason: whoever uses kustomize edit is more expecting kustomize to give guidance and validation checks. Thus, validation checks like valid yaml, valid OpenApi could always be super helpful. We can add a TODO here if not planning to add the validation check for now. But overall I feel like expanding the validation checks on kustomize edit is a good approach to introduce users to rely more on this command.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I saw the GlobPatterns below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To your first point, yeah the validation of the path is done in GlobPatterns. But I think it makes more sense to do that step in Validate, so I've moved it over.v

For the second point, I think that's a good idea but would be better in its own PR, so I added a TODO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sg.

@yuwenma
Copy link
Contributor

yuwenma commented Jan 5, 2022

/lgtm
👍 for the transformer rewording.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2022
@k8s-ci-robot k8s-ci-robot merged commit 37668d8 into kubernetes-sigs:master Jan 5, 2022
@ephesused
Copy link
Contributor

ephesused commented Jan 13, 2022

Edited to add:

The below analysis is incorrect. This PR is not the cause of the sorting change described in the remainder of this comment.


For awareness, this commit slightly alters name sorting for resources. Previously (ff7b2f2) kustomize would order like so:

  1. testing
  2. testing-one
  3. testing-two

With this change (92197fd) kustomize orders like so:

  1. testing-one
  2. testing-two
  3. testing

Test case files

kustomization.yaml:

resources:
  - resources.yaml

resources.yaml:

apiVersion: v1
kind: ConfigMap
metadata:
  name: testing
data:
  key: value
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: testing-one
data:
  key: value
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: testing-two
data:
  key: value

@KnVerey
Copy link
Contributor

KnVerey commented Jan 13, 2022

@ephesused can you please open an issue with reproduction steps including Kustomize versions. A change in order is concerning, but I don't think this PR could have caused it. It is unreleased, and only affects a command that helps you author the Kustomization itself; it does not affect Kustomize buid.

@ephesused
Copy link
Contributor

I'll open a separate issue, but this isn't an change I'm seeing in any releases. I'm seeing the difference on the master branch, between commits ff7b2f2 and 92197fd. I had thought 92197fd was this PR.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Dec 6, 2022
Changes:
4.5.7
-----
Due to an oversight, kustomize v4.5.6 has the golang testing library
compiled in unnecessarily. This is a rerelease with the same
functionality, but without the unnecessary additional library compiled
in.

4.5.6
-----
Due to an oversight, kustomize v4.5.6 has the golang testing library
compiled in unnecessarily. It is advised that you upgrade to v4.5.7,
which doesn't have the testing library compiled in.

4.5.5
-----
This release is expected to have significant performance improvements
for a good portion of inputs, due to #4568.

4.5.4
-----
Dependencies updates.

4.5.3
-----
Misc enhancements and bug fixes.

4.5.2
-----
Bug fix release.

4.5.1
-----
Known issues:
 - kubernetes-sigs/kustomize#4455
   A regression, some HTTP urls are not working properly.

4.5.0
-----
 - This release contains a regression in the legacy sort order. Those
   using the legacy sort, i.e. `kustomize build` with `--reorder` unset
   or explicitly set to `legacy`, are advised to skip this release.
   kubernetes-sigs/kustomize#4388
 - kubernetes-sigs/kustomize#4455
   Another regression, some HTTP urls are not working properly.

 - New field in kustomization, `buildMetadata`.
 - New command `kustomize edit add buildmetadata`
 - Refactor the PrefixSuffixTransformer into separate prefix- and
   suffix transformers, enabling the user to use the PrefixTransformer or
   SuffixTransformer individually in the transformers field.
 - `kustomize build ...` now completes file paths on ZSH.
 - New command `kustomize edit add generator` (kubernetes-sigs/kustomize#4361)

 - Deprecate enable-managedby-label flag in favor of a field

4.4.1
------
Bug fix release.

4.4.0
-----
The headline feature of this release is improved support for YAML
anchors and aliases, which will be expanded by default as of this
version.

Additional features and fixes include:
- Added length check on originalFields of kustomizationFile to prevent
  panic when kustomization file began with a comment(or a blank line)
  followed by a document separator

4.3.0
-----
Misc bug fixes and enhancements.

4.2.0
-----
New experimental command to automatically migrate `vars` to
`replacements`: `kustomize edit fix —vars`. For details, run `kustomize
edit fix -h`. Warning: converting `vars` to `replacements` will
potentially overwrite many resource files and in rare scenarios may not
produce the same output when `kustomize build` is run. We recommend
doing this in a clean git repository where the change is easy to undo.

4.1.3
-----
* New experimental ReplacementTransformer, docs on the way:
  kubernetes-sigs/cli-experimental#158
  This will replace the `vars` feature.
* Allow pulls of openapi data from live API servers (openapi fetch command).
* Remote git urls can specify a timeout parameter.
* More examples of helm usage.
* Speed up cluster-scoped type checks.
* API changes towards 1.0
  - `Gvk` and `Resid` types moved to kyaml
  - `Resource` now inlines `RNode` rather than delegating to it
  - `Resmap` now accepts an `kio.Filter` visitor (that can change the ResMap size).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kustomize edit add generator
5 participants