From 33b7432d6a10fdd3b81898e886612d9287d14ccf Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Fri, 1 Dec 2017 01:47:58 -0500 Subject: [PATCH] Fix segfault when updating non-existent object Fix #3935 --- cmd/kops/replace.go | 38 ++++++++++++++++++----- docs/cli/kops_replace.md | 2 +- pkg/client/simple/vfsclientset/cluster.go | 7 +++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/cmd/kops/replace.go b/cmd/kops/replace.go index 15b34181b4741..83588c4837661 100644 --- a/cmd/kops/replace.go +++ b/cmd/kops/replace.go @@ -23,12 +23,12 @@ import ( "github.com/golang/glog" "github.com/spf13/cobra" - "k8s.io/kops/cmd/kops/util" - "k8s.io/kops/util/pkg/vfs" - + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kops/cmd/kops/util" kopsapi "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/kopscodecs" + "k8s.io/kops/util/pkg/vfs" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" @@ -77,7 +77,7 @@ func NewCmdReplace(f *util.Factory, out io.Writer) *cobra.Command { }, } cmd.Flags().StringSliceVarP(&options.Filenames, "filename", "f", options.Filenames, "A list of one or more files separated by a comma.") - cmd.Flags().BoolVarP(&options.force, "force", "", false, "Force any changes, which will also create any non-existing respurce (defaults to instancegroups only)") + cmd.Flags().BoolVarP(&options.force, "force", "", false, "Force any changes, which will also create any non-existing resource") cmd.MarkFlagRequired("filename") return cmd @@ -124,9 +124,29 @@ func RunReplace(f *util.Factory, cmd *cobra.Command, out io.Writer, c *replaceOp return err } - _, err = clientset.UpdateCluster(v, status) + // Check if the cluster exists already + clusterName := v.Name + cluster, err := clientset.GetCluster(clusterName) if err != nil { - return fmt.Errorf("error replacing cluster: %v", err) + if errors.IsNotFound(err) { + cluster = nil + } else { + return fmt.Errorf("error fetching cluster %q: %v", clusterName, err) + } + } + if cluster == nil { + if !c.force { + return fmt.Errorf("cluster %v does not exist (try adding --force flag)", clusterName) + } + _, err = clientset.CreateCluster(v) + if err != nil { + return fmt.Errorf("error creating cluster: %v", err) + } + } else { + _, err = clientset.UpdateCluster(v, status) + if err != nil { + return fmt.Errorf("error replacing cluster: %v", err) + } } } @@ -137,7 +157,11 @@ func RunReplace(f *util.Factory, cmd *cobra.Command, out io.Writer, c *replaceOp } cluster, err := clientset.GetCluster(clusterName) if err != nil { - return fmt.Errorf("error fetching cluster %q: %v", clusterName, err) + if errors.IsNotFound(err) { + cluster = nil + } else { + return fmt.Errorf("error fetching cluster %q: %v", clusterName, err) + } } if cluster == nil { return fmt.Errorf("cluster %q not found", clusterName) diff --git a/docs/cli/kops_replace.md b/docs/cli/kops_replace.md index ecb6096eb5716..65208b5e1ef09 100644 --- a/docs/cli/kops_replace.md +++ b/docs/cli/kops_replace.md @@ -28,7 +28,7 @@ kops replace -f FILENAME ``` -f, --filename stringSlice A list of one or more files separated by a comma. - --force Force any changes, which will also create any non-existing respurce (defaults to instancegroups only) + --force Force any changes, which will also create any non-existing resource ``` ### Options inherited from parent commands diff --git a/pkg/client/simple/vfsclientset/cluster.go b/pkg/client/simple/vfsclientset/cluster.go index de74b21f5b8af..9bfd8efa5fe47 100644 --- a/pkg/client/simple/vfsclientset/cluster.go +++ b/pkg/client/simple/vfsclientset/cluster.go @@ -23,7 +23,9 @@ import ( "time" "github.com/golang/glog" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/watch" @@ -50,6 +52,7 @@ func (c *ClusterVFS) Get(name string, options metav1.GetOptions) (*api.Cluster, if options.ResourceVersion != "" { return nil, fmt.Errorf("ResourceVersion not supported in ClusterVFS::Get") } + // TODO: Return not found return c.find(name) } @@ -123,6 +126,10 @@ func (r *ClusterVFS) Update(c *api.Cluster, status *api.ClusterStatus) (*api.Clu return nil, err } + if old == nil { + return nil, errors.NewNotFound(schema.GroupResource{Group: api.GroupName, Resource: "Cluster"}, clusterName) + } + if err := validation.ValidateClusterUpdate(c, status, old).ToAggregate(); err != nil { return nil, err }