mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-21 10:51:29 +00:00
Merge pull request #89799 from julianvmodesto/patcher
Make kubectl client-side apply with server-side dry-run safer
This commit is contained in:
commit
8b3c00c661
@ -68,7 +68,6 @@ go_test(
|
||||
"//staging/src/k8s.io/client-go/dynamic/fake:go_default_library",
|
||||
"//staging/src/k8s.io/client-go/rest:go_default_library",
|
||||
"//staging/src/k8s.io/client-go/rest/fake:go_default_library",
|
||||
"//staging/src/k8s.io/client-go/testing:go_default_library",
|
||||
"//staging/src/k8s.io/kubectl/pkg/cmd/testing:go_default_library",
|
||||
"//staging/src/k8s.io/kubectl/pkg/cmd/util:go_default_library",
|
||||
"//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library",
|
||||
|
@ -407,6 +407,20 @@ func (o *ApplyOptions) applyOneObject(info *resource.Info) error {
|
||||
klog.V(4).Infof("error recording current command: %v", err)
|
||||
}
|
||||
|
||||
helper := resource.NewHelper(info.Client, info.Mapping).
|
||||
DryRun(o.DryRunStrategy == cmdutil.DryRunServer).
|
||||
WithFieldManager(o.FieldManager)
|
||||
|
||||
if o.DryRunStrategy == cmdutil.DryRunServer {
|
||||
// Ensure the APIServer supports server-side dry-run for the resource,
|
||||
// otherwise fail early.
|
||||
// For APIServers that don't support server-side dry-run will persist
|
||||
// changes.
|
||||
if err := o.DryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if o.ServerSideApply {
|
||||
// Send the full object to be applied on the server side.
|
||||
data, err := runtime.Encode(unstructured.UnstructuredJSONScheme, info.Object)
|
||||
@ -417,14 +431,6 @@ func (o *ApplyOptions) applyOneObject(info *resource.Info) error {
|
||||
options := metav1.PatchOptions{
|
||||
Force: &o.ForceConflicts,
|
||||
}
|
||||
helper := resource.NewHelper(info.Client, info.Mapping).
|
||||
WithFieldManager(o.FieldManager)
|
||||
if o.DryRunStrategy == cmdutil.DryRunServer {
|
||||
if err := o.DryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil {
|
||||
return err
|
||||
}
|
||||
helper.DryRun(true)
|
||||
}
|
||||
obj, err := helper.Patch(
|
||||
info.Namespace,
|
||||
info.Name,
|
||||
@ -495,14 +501,6 @@ See http://k8s.io/docs/reference/using-api/api-concepts/#conflicts`, err)
|
||||
|
||||
if o.DryRunStrategy != cmdutil.DryRunClient {
|
||||
// Then create the resource and skip the three-way merge
|
||||
helper := resource.NewHelper(info.Client, info.Mapping).
|
||||
WithFieldManager(o.FieldManager)
|
||||
if o.DryRunStrategy == cmdutil.DryRunServer {
|
||||
if err := o.DryRunVerifier.HasSupport(info.Mapping.GroupVersionKind); err != nil {
|
||||
return cmdutil.AddSourceToErr("creating", info.Source, err)
|
||||
}
|
||||
helper.DryRun(true)
|
||||
}
|
||||
obj, err := helper.Create(info.Namespace, true, info.Object)
|
||||
if err != nil {
|
||||
return cmdutil.AddSourceToErr("creating", info.Source, err)
|
||||
@ -539,7 +537,7 @@ See http://k8s.io/docs/reference/using-api/api-concepts/#conflicts`, err)
|
||||
fmt.Fprintf(o.ErrOut, warningNoLastAppliedConfigAnnotation, o.cmdBaseName)
|
||||
}
|
||||
|
||||
patcher, err := newPatcher(o, info)
|
||||
patcher, err := newPatcher(o, info, helper)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
@ -44,7 +44,6 @@ import (
|
||||
dynamicfakeclient "k8s.io/client-go/dynamic/fake"
|
||||
restclient "k8s.io/client-go/rest"
|
||||
"k8s.io/client-go/rest/fake"
|
||||
clienttesting "k8s.io/client-go/testing"
|
||||
cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
|
||||
cmdutil "k8s.io/kubectl/pkg/cmd/util"
|
||||
"k8s.io/kubectl/pkg/scheme"
|
||||
@ -1333,6 +1332,11 @@ func TestForceApply(t *testing.T) {
|
||||
}
|
||||
t.Fatalf("unexpected request: %#v after %v tries\n%#v", req.URL, counts["patch"], req)
|
||||
return nil, nil
|
||||
case strings.HasSuffix(p, pathRC) && m == "DELETE":
|
||||
counts["delete"]++
|
||||
deleted = true
|
||||
bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC))
|
||||
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil
|
||||
case strings.HasSuffix(p, pathRC) && m == "PUT":
|
||||
counts["put"]++
|
||||
bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC))
|
||||
@ -1351,16 +1355,6 @@ func TestForceApply(t *testing.T) {
|
||||
}),
|
||||
}
|
||||
fakeDynamicClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
|
||||
fakeDynamicClient.PrependReactor("delete", "replicationcontrollers", func(action clienttesting.Action) (bool, runtime.Object, error) {
|
||||
if deleteAction, ok := action.(clienttesting.DeleteAction); ok {
|
||||
if deleteAction.GetName() == nameRC {
|
||||
counts["delete"]++
|
||||
deleted = true
|
||||
return true, nil, nil
|
||||
}
|
||||
}
|
||||
return false, nil, nil
|
||||
})
|
||||
tf.FakeDynamicClient = fakeDynamicClient
|
||||
tf.OpenAPISchemaFunc = fn
|
||||
tf.Client = tf.UnstructuredClient
|
||||
|
@ -33,7 +33,6 @@ import (
|
||||
"k8s.io/apimachinery/pkg/util/strategicpatch"
|
||||
"k8s.io/apimachinery/pkg/util/wait"
|
||||
"k8s.io/cli-runtime/pkg/resource"
|
||||
"k8s.io/client-go/dynamic"
|
||||
oapi "k8s.io/kube-openapi/pkg/util/proto"
|
||||
cmdutil "k8s.io/kubectl/pkg/cmd/util"
|
||||
"k8s.io/kubectl/pkg/scheme"
|
||||
@ -54,7 +53,6 @@ const (
|
||||
type Patcher struct {
|
||||
Mapping *meta.RESTMapping
|
||||
Helper *resource.Helper
|
||||
DynamicClient dynamic.Interface
|
||||
|
||||
Overwrite bool
|
||||
BackOff clockwork.Clock
|
||||
@ -63,7 +61,6 @@ type Patcher struct {
|
||||
Cascade bool
|
||||
Timeout time.Duration
|
||||
GracePeriod int
|
||||
ServerDryRun bool
|
||||
|
||||
// If set, forces the patch against a specific resourceVersion
|
||||
ResourceVersion *string
|
||||
@ -74,7 +71,7 @@ type Patcher struct {
|
||||
OpenapiSchema openapi.Resources
|
||||
}
|
||||
|
||||
func newPatcher(o *ApplyOptions, info *resource.Info) (*Patcher, error) {
|
||||
func newPatcher(o *ApplyOptions, info *resource.Info, helper *resource.Helper) (*Patcher, error) {
|
||||
var openapiSchema openapi.Resources
|
||||
if o.OpenAPIPatch {
|
||||
openapiSchema = o.OpenAPISchema
|
||||
@ -82,22 +79,22 @@ func newPatcher(o *ApplyOptions, info *resource.Info) (*Patcher, error) {
|
||||
|
||||
return &Patcher{
|
||||
Mapping: info.Mapping,
|
||||
Helper: resource.NewHelper(info.Client, info.Mapping).WithFieldManager(o.FieldManager),
|
||||
DynamicClient: o.DynamicClient,
|
||||
Helper: helper,
|
||||
Overwrite: o.Overwrite,
|
||||
BackOff: clockwork.NewRealClock(),
|
||||
Force: o.DeleteOptions.ForceDeletion,
|
||||
Cascade: o.DeleteOptions.Cascade,
|
||||
Timeout: o.DeleteOptions.Timeout,
|
||||
GracePeriod: o.DeleteOptions.GracePeriod,
|
||||
ServerDryRun: o.DryRunStrategy == cmdutil.DryRunServer,
|
||||
OpenapiSchema: openapiSchema,
|
||||
Retries: maxPatchRetry,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (p *Patcher) delete(namespace, name string) error {
|
||||
return runDelete(namespace, name, p.Mapping, p.DynamicClient, p.Cascade, p.GracePeriod, p.ServerDryRun)
|
||||
options := asDeleteOptions(p.Cascade, p.GracePeriod)
|
||||
_, err := p.Helper.DeleteWithOptions(namespace, name, &options)
|
||||
return err
|
||||
}
|
||||
|
||||
func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, namespace, name string, errOut io.Writer) ([]byte, runtime.Object, error) {
|
||||
@ -178,7 +175,7 @@ func (p *Patcher) patchSimple(obj runtime.Object, modified []byte, source, names
|
||||
}
|
||||
}
|
||||
|
||||
patchedObj, err := p.Helper.DryRun(p.ServerDryRun).Patch(namespace, name, patchType, patch, nil)
|
||||
patchedObj, err := p.Helper.Patch(namespace, name, patchType, patch, nil)
|
||||
return patch, patchedObj, err
|
||||
}
|
||||
|
||||
@ -223,11 +220,11 @@ func (p *Patcher) deleteAndCreate(original runtime.Object, modified []byte, name
|
||||
if err != nil {
|
||||
return modified, nil, err
|
||||
}
|
||||
createdObject, err := p.Helper.DryRun(p.ServerDryRun).Create(namespace, true, versionedObject)
|
||||
createdObject, err := p.Helper.Create(namespace, true, versionedObject)
|
||||
if err != nil {
|
||||
// restore the original object if we fail to create the new one
|
||||
// but still propagate and advertise error to user
|
||||
recreated, recreateErr := p.Helper.DryRun(p.ServerDryRun).Create(namespace, true, original)
|
||||
recreated, recreateErr := p.Helper.Create(namespace, true, original)
|
||||
if recreateErr != nil {
|
||||
err = fmt.Errorf("An error occurred force-replacing the existing object with the newly provided one:\n\n%v.\n\nAdditionally, an error occurred attempting to restore the original object:\n\n%v", err, recreateErr)
|
||||
} else {
|
||||
|
@ -143,19 +143,24 @@ func (p *pruner) delete(namespace, name string, mapping *meta.RESTMapping) error
|
||||
}
|
||||
|
||||
func runDelete(namespace, name string, mapping *meta.RESTMapping, c dynamic.Interface, cascade bool, gracePeriod int, serverDryRun bool) error {
|
||||
options := asDeleteOptions(cascade, gracePeriod)
|
||||
if serverDryRun {
|
||||
options.DryRun = []string{metav1.DryRunAll}
|
||||
}
|
||||
return c.Resource(mapping.Resource).Namespace(namespace).Delete(context.TODO(), name, options)
|
||||
}
|
||||
|
||||
func asDeleteOptions(cascade bool, gracePeriod int) metav1.DeleteOptions {
|
||||
options := metav1.DeleteOptions{}
|
||||
if gracePeriod >= 0 {
|
||||
options = *metav1.NewDeleteOptions(int64(gracePeriod))
|
||||
}
|
||||
if serverDryRun {
|
||||
options.DryRun = []string{metav1.DryRunAll}
|
||||
}
|
||||
policy := metav1.DeletePropagationForeground
|
||||
if !cascade {
|
||||
policy = metav1.DeletePropagationOrphan
|
||||
}
|
||||
options.PropagationPolicy = &policy
|
||||
return c.Resource(mapping.Resource).Namespace(namespace).Delete(context.TODO(), name, options)
|
||||
return options
|
||||
}
|
||||
|
||||
type pruneResource struct {
|
||||
|
@ -367,7 +367,6 @@ func (obj InfoObject) Merged() (runtime.Object, error) {
|
||||
Helper: helper,
|
||||
Overwrite: true,
|
||||
BackOff: clockwork.NewRealClock(),
|
||||
ServerDryRun: true,
|
||||
OpenapiSchema: obj.OpenAPI,
|
||||
ResourceVersion: resourceVersion,
|
||||
}
|
||||
|
@ -523,8 +523,23 @@ func GetFieldManagerFlag(cmd *cobra.Command) string {
|
||||
type DryRunStrategy int
|
||||
|
||||
const (
|
||||
// DryRunNone indicates the client will make all mutating calls
|
||||
DryRunNone DryRunStrategy = iota
|
||||
|
||||
// DryRunClient, or client-side dry-run, indicates the client will prevent
|
||||
// making mutating calls such as CREATE, PATCH, and DELETE
|
||||
DryRunClient
|
||||
|
||||
// DryRunServer, or server-side dry-run, indicates the client will send
|
||||
// mutating calls to the APIServer with the dry-run parameter to prevent
|
||||
// persisting changes.
|
||||
//
|
||||
// Note that clients sending server-side dry-run calls should verify that
|
||||
// the APIServer and the resource supports server-side dry-run, and otherwise
|
||||
// clients should fail early.
|
||||
//
|
||||
// If a client sends a server-side dry-run call to an APIServer that doesn't
|
||||
// support server-side dry-run, then the APIServer will persist changes inadvertently.
|
||||
DryRunServer
|
||||
)
|
||||
|
||||
|
@ -109,10 +109,15 @@ run_kubectl_apply_tests() {
|
||||
kube::test::get_object_assert pods "{{range.items}}{{${id_field:?}}}:{{end}}" ''
|
||||
# apply non dry-run creates the pod
|
||||
kubectl apply -f hack/testdata/pod.yaml "${kube_flags[@]:?}"
|
||||
initialResourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}')
|
||||
# apply changes
|
||||
kubectl apply --dry-run=client -f hack/testdata/pod-apply.yaml "${kube_flags[@]:?}"
|
||||
kubectl apply --dry-run=server -f hack/testdata/pod-apply.yaml "${kube_flags[@]:?}"
|
||||
# Post-Condition: label still has initial value
|
||||
kube::test::get_object_assert 'pods test-pod' "{{${labels_field:?}.name}}" 'test-pod-label'
|
||||
# Ensure dry-run doesn't persist change
|
||||
resourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}')
|
||||
kube::test::if_has_string "${resourceVersion}" "${initialResourceVersion}"
|
||||
|
||||
# clean-up
|
||||
kubectl delete -f hack/testdata/pod.yaml "${kube_flags[@]:?}"
|
||||
@ -377,10 +382,14 @@ run_kubectl_server_side_apply_tests() {
|
||||
kube::test::get_object_assert pods "{{range.items}}{{${id_field:?}}}:{{end}}" ''
|
||||
# apply non dry-run creates the pod
|
||||
kubectl apply --server-side -f hack/testdata/pod.yaml "${kube_flags[@]:?}"
|
||||
initialResourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}')
|
||||
# apply changes
|
||||
kubectl apply --server-side --dry-run=server -f hack/testdata/pod-apply.yaml "${kube_flags[@]:?}"
|
||||
# Post-Condition: label still has initial value
|
||||
kube::test::get_object_assert 'pods test-pod' "{{${labels_field:?}.name}}" 'test-pod-label'
|
||||
# Ensure dry-run doesn't persist change
|
||||
resourceVersion=$(kubectl get "${kube_flags[@]:?}" -f hack/testdata/pod.yaml -o go-template='{{ .metadata.resourceVersion }}')
|
||||
kube::test::if_has_string "${resourceVersion}" "${initialResourceVersion}"
|
||||
|
||||
# clean-up
|
||||
kubectl delete -f hack/testdata/pod.yaml "${kube_flags[@]:?}"
|
||||
|
Loading…
Reference in New Issue
Block a user