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

Remove fix kustomization step before Unmarshalling the yaml structure #4923

Conversation

koba1t
Copy link
Member

@koba1t koba1t commented Dec 9, 2022

This has already been talked about at #4723 (comment)

related PR

/cc @KnVerey @natasha41575

@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 9, 2022
@koba1t
Copy link
Member Author

koba1t commented Dec 9, 2022

/cc @natasha41575

@natasha41575 natasha41575 added this to the v5.0.0 milestone Dec 9, 2022
Copy link
Contributor

@natasha41575 natasha41575 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! This seems like a reasonable way to handle imageTags. I'm holding off on approving so that @KnVerey can take a look too.

@annasong20 With this change, localize will also stop auto-converting imageTags to images, which will be really nice.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 9, 2022
@koba1t koba1t changed the title Remove fix kustomization step before Unmarshalling the yaml structure. Remove fix kustomization step before Unmarshalling the yaml structure Dec 9, 2022
@natasha41575
Copy link
Contributor

natasha41575 commented Dec 13, 2022

@koba1t when #4918 is merged, could you rebase this PR? #4918 makes a call toFixKustomizationPreUnmarshalling, but you can just remove those lines after you rebase and update any failing tests accordingly. Feel free to reach out if this isn't clear or if you run into any other problems with the localizer code.

@annasong20
Copy link
Contributor

annasong20 commented Dec 13, 2022

@koba1t In #4918, same PR that @natasha41575 mentions above, images in this line: https://github.com/kubernetes-sigs/kustomize/pull/4918/files#diff-c21f7b3d0afda4a64163411e90e72d2fed6c3df14c7b6f8ed07237bb5e1fc91cR213 will also need to be changed to imageTags. Feel free to ping me with any problems.

@koba1t koba1t force-pushed the chore/remove_FixKustomizationPreUnmarshalling branch from 8a99b75 to 52faaf8 Compare December 13, 2022 18:29
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2022
@koba1t koba1t force-pushed the chore/remove_FixKustomizationPreUnmarshalling branch from 52faaf8 to faddb49 Compare December 13, 2022 18:50
@koba1t
Copy link
Member Author

koba1t commented Dec 13, 2022

@natasha41575 @annasong20
I was done to rebase master and fix at #4923 (comment) line.
All tests and lint were passed.

Could you check it?

Copy link
Contributor

@natasha41575 natasha41575 left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2022
@natasha41575
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: koba1t, 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 13, 2022
@k8s-ci-robot k8s-ci-robot merged commit e3981da into kubernetes-sigs:master Dec 13, 2022
@koba1t koba1t deleted the chore/remove_FixKustomizationPreUnmarshalling branch December 13, 2022 20:13
waleedhammam added a commit to weaveworks/weave-policy-validator that referenced this pull request May 13, 2023
waleedhammam added a commit to weaveworks/weave-policy-validator that referenced this pull request May 13, 2023
waleedhammam added a commit to weaveworks/weave-policy-validator that referenced this pull request May 13, 2023
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.

None yet

4 participants