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

fix: patch additions honor source key style #5005

Closed

Conversation

ephesused
Copy link
Contributor

When a patch appends a new node, it should honor the key style from the patch. Prior to this commit, no style was applied, leading to problems when the key value could be interpreted as a different type based on its content. For example, "9110" needs quoting to ensure it is seen as a string in yaml.

Fixes #4928

koba1t and others added 2 commits December 9, 2022 18:14
When a patch appends a new node, it should honor the key style from the
patch. Prior to this commit, no style was applied, leading to problems
when the key value could be interpreted as a different type based on its
content. For example, "9110" needs quoting to ensure it is seen as a
string in yaml.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 28, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @ephesused. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 28, 2023
KnVerey and others added 21 commits January 30, 2023 15:48
…al-releases

Warn against partial releases
* Localize HelmChartInflationGenerator

* Add explicit inline generators test
…ze-dst

Expose path to `localize` destination
* Remove go module ci job

* Add script that runs go mod tidy with replace statements

* Invoke one script from the makefile and pass in the command to run in the pinned context

---------

Co-authored-by: Anna Song <annasong@google.com>
make TestResourcesRepoNotFile even less specific
* Add localize command handle

* Align to kustomize command conventions

* Print success msg
* support for more helm template args

* move templateArgs and unit tests to api/types

* undo package name change

* use our own simple helm chart instead of forking one

* add argument to AsHelmArgs

* code review

* lint errors
* Update Versioning to Improve Output

* Always get commit from build info, always get date and version from ldflag

* Just replace broken main output with semver and deprecate short flag as is

---------

Co-authored-by: Katrina Verey <katrina.verey@shopify.com>
Allow authenticated Github use in changelog script and improve error messages
…authed

Don't have empty string when no auth info
@ephesused
Copy link
Contributor Author

@ephesused did you have any chance to work on this?

Apologies; I have not. I'll make a concerted effort to get back to it in the next week.

@ephesused
Copy link
Contributor Author

Apologies, I'm not going to get to it this week. We had a number of high priority issues come through, and I haven't had the time. I will try to address this one next week.

k8s-ci-robot and others added 5 commits May 26, 2023 10:36
…tomization_is_empty

add check that kustomization is empty
Update gnostic dependency with gnostic-models
kubernetes-sigs#5170)

* change: components apply after all generator and transformer applied

* fix name for a test case

* add comment about when the components will be executed

* components are applied before transformer
@Hammond95
Copy link

Hi @ephesused, thank you for your work 🙏

@ephesused
Copy link
Contributor Author

After a very busy few weeks, things are clearing up. I will make progress on this PR tomorrow.

k8s-ci-robot and others added 6 commits June 7, 2023 07:42
…_path_match

be error when no path matching
When a patch appends a new node, it should honor the key style from the
patch. Prior to this commit, no style was applied, leading to problems
when the key value could be interpreted as a different type based on its
content. For example, "9110" needs quoting to ensure it is seen as a
string in yaml.
@k8s-ci-robot
Copy link
Contributor

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.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ephesused
Once this PR has been reviewed and has the lgtm label, please assign natasha41575 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 8, 2023
@ephesused
Copy link
Contributor Author

ephesused commented Jun 8, 2023

Whoops - I mistakenly mismanaged my branch. Apologies. If it's simpler, I can abandon this PR and open a clean one.

Edited to add: Yes, I think that's what I'm going to do. Sorry for the hassle.

@ephesused
Copy link
Contributor Author

This PR is superseded by #5196. Sorry for the trouble.

@ephesused ephesused closed this Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kustomize fails to apply overlay if key in base yaml is numeric and missing