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

drop support for legacy patches #4911

Merged

Conversation

natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Dec 6, 2022

/cc @KnVerey

close #4910

This PR drops support for a very old style of patches. "Legacy patches" refers to being able to supply in the patches field what patchesStrategicMerge does now. We are not dropping support for any of the documented fields.

For example, you used to be able to do:

patches:
- |-
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    name: nginx
  spec:
    template:
      spec:
        containers:
          - name: nginx
            image: nignx:latest 

This is very very old syntax. We are dropping support for this legacy style which we haven't documented in a very long time.

Now, you have to use patches as documented, which means either:

patchesStrategicMerge:
- |-
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    name: nginx
  spec:
    template:
      spec:
        containers:
          - name: nginx
            image: nignx:latest 

or

patches:
- patch: |-
      apiVersion: apps/v1
      kind: Deployment
      metadata:
        name: nginx
      spec:
        template:
          spec:
            containers:
              - name: nginx
                image: nignx:latest 

@natasha41575 natasha41575 added this to the v5.0.0 milestone Dec 6, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 6, 2022
@k8s-ci-robot
Copy link
Contributor

@natasha41575: 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 natasha41575 added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 7, 2022
@@ -40,7 +40,7 @@ func TestLoad(t *testing.T) {
},
},
"nonsenseLatin": {
errContains: "error converting YAML to JSON",
errContains: "found a tab character that violates indentation",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this points out the test isn't testing what it claimed, i.e. it is testing tab indent vs space indent instead of random text vs yaml. The real error in a nonsense case is fine too (yaml: line 6: could not find expected ':') and also an improvement on the misleading error we gave before!

@@ -503,7 +503,7 @@ spec:
`)

// component declared in overlay with custom schema and patch
th.WriteC("components/dc-openapi", `patches:
th.WriteC("components/dc-openapi", `patchesStrategicMerge:
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

@KnVerey KnVerey added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label Dec 8, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KnVerey, 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 [KnVerey,natasha41575]

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

@KnVerey
Copy link
Contributor

KnVerey commented Dec 8, 2022

/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 8, 2022
@k8s-ci-robot k8s-ci-robot merged commit 2db573b into kubernetes-sigs:master Dec 8, 2022
elisshafer pushed a commit to elisshafer/kustomize that referenced this pull request Dec 8, 2022
* drop support for legacy patches

* fix CI
@natasha41575 natasha41575 deleted the dropLegacyPatchSupport branch December 8, 2022 17:38
cailynse pushed a commit to cailynse/kustomize that referenced this pull request Jan 12, 2023
* drop support for legacy patches

* fix CI
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 1, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 1, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 5, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 5, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 10, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
garloff added a commit to SovereignCloudStack/k8s-cluster-api-provider that referenced this pull request Mar 10, 2023
Use resources rather than base.
Insert path: before filenames referenced in patches.
Reference: kubernetes-sigs/kustomize#4911
Supercedes: #329

Signed-off-by: Kurt Garloff <kurt@garloff.de>
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Apr 15, 2023
v1.27.1

No CLI change.

v1.27.0

API Change

* Adds feature gate NodeLogQuery which provides cluster administrators with
a streaming view of logs using kubectl without them having to implement a
client side reader or logging into the node.

Feature

* Added "general", "baseline", and "restricted" debugging profiles for
kubectl debug.
* Added "netadmin" debugging profiles for kubectl debug.
* Added --output plaintext-openapiv2 argument to kubectl explain to use old
openapiv2 explain implementation.
* Added e2e tests for kubectl --subresource for beta graduation
* Changed kubectl --subresource flag to beta (#116595, @MadhavJivrajani)
* Enable external plugins can be used as subcommands for kubectl create
command if subcommand does not exist as builtin only when
KUBECTL_ENABLE_CMD_SHADOW environment variable is exported.
* Kubectl will now display SeccompProfile for pods, containers and
ephemeral containers, if values were set.
* Kubectl: added e2e test for default container annotation
* Made kubectl-convert binary linking static (also affects the deb and rpm
packages).
* Promoted whoami kubectl command.
* Since Kubernetes v1.5, kubectl apply has had an alpha-stage --prune flag
to support deleting previously applied objects that have been removed from
the input manifest. This feature has remained in alpha ever since due to
performance and correctness issues inherent in its design. This PR exposes
a second, independent pruning alpha powered by a new standard named
ApplySets. An ApplySet is a server-side object (by default, a Secret;
ConfigMaps are also allowed) that kubectl can use to accurately and
efficiently track set membership across apply operations. The format used
for ApplySet is set out in KEP 3659 as a low-level specification. Other
tools in the ecosystem can also build on this specification for improved
interoperability. To try the ApplySet-based pruning alpha, set
KUBECTL_APPLYSET=true and use the flags --prune --applyset=secret-name with
kubectl apply.
* Switched kubectl explain to use OpenAPIV3 information published by the
server. OpenAPIV2 backend can still be used with the --output
plaintext-openapiv2 argument
* Upgrades functionality of kubectl kustomize as described at
https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.0
and
https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.1.
This is a new major release of kustomize, so there are a few
backwards-incompatible changes, most of which are rare use cases, bug fixes
with side effects, or things that have been deprecated for multiple
releases already:

 - kubernetes-sigs/kustomize#4911: Drop support for a very old, legacy
 style of patches. patches used to be allowed to be used as an alias for
 patchesStrategicMerge in kustomize v3. You now have to use
 patchesStrategicMerge explicitly, or update to the new syntax supported by
 patches. See examples in the PR description of
 kubernetes-sigs/kustomize#4911.
 - kubernetes-sigs/kustomize#4731: Remove a potential build-time
 side-effect in ConfigMapGenerator and SecretGenerator, which loaded values
 from the local environment under some circumstances, breaking kustomize
 build's side-effect-free promise. While this behavior was never intended,
 we deprecated it and are announcing it as a breaking change since it
 existed for a long time. See also the Eschewed Features documentation.
 - kubernetes-sigs/kustomize#4985: If you previously included .git in an
 AWS or Azure URL, we will no longer automatically remove that suffix. You
 may need to add an extra / to replace the .git for the URL to properly
 resolve.
 - kubernetes-sigs/kustomize#4954: Drop support for using gh: as a
 host (e.g. gh:kubernetes-sigs/kustomize). We were unable to find any usage
 of or basis for this and believe it may have been targeting a custom
 gitconfig shorthand syntax.

* [alpha: kubectl apply --prune --applyset] Enabled certain custom
resources (CRs) to be used as ApplySet parent objects. To enable this for a
given CR, apply the label applyset.kubernetes.io/is-parent-type: true to
the CustomResourceDefinition (CRD) that defines it.
* kubectl now uses HorizontalPodAutoscaler v2 by default.

Documentation

* kubectl create rolebinding -h

Bug or Regression

* Added (dry run) and (server dry run) suffixes to kubectl scale command
when dry-run is passed
* Changed the error message of kubectl rollout restart when subsequent
kubectl rollout restart commands are executed within a second
* Changed the error message to cannot exec into multiple objects at a time
when file passed to kubectl exec contains multiple resources
* Kubectl: enabled usage of label selector for filtering out resources when
pruning for kubectl diff
* kubectl port-forward now exits with exit code 1 when remote connection is
lost
halvards added a commit to halvards/skaffold that referenced this pull request May 8, 2023
This change replaces the obsolete use of the Kustomize `patches` field
in the Skaffold examples with `patchesStrategicMerge`.

Context:
Kustomize v5 removed support for `patches` being an array of strings.

The `patchesStrategicMerge` field introduced in Kustomize v3 retains the
previous behavior of the `patches`.

The `patches` field can still be used in v5 with an array of `patch`
objects.

Additional information:
- kubernetes-sigs/kustomize#4911
- https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.0
ericzzzzzzz pushed a commit to GoogleContainerTools/skaffold that referenced this pull request May 31, 2023
This change replaces the obsolete use of the Kustomize `patches` field
in the Skaffold examples with `patchesStrategicMerge`.

Context:
Kustomize v5 removed support for `patches` being an array of strings.

The `patchesStrategicMerge` field introduced in Kustomize v3 retains the
previous behavior of the `patches`.

The `patches` field can still be used in v5 with an array of `patch`
objects.

Additional information:
- kubernetes-sigs/kustomize#4911
- https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.0
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. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop support for legacy patches
3 participants