Skip to content

Commit

Permalink
Fix segfault when updating non-existent object
Browse files Browse the repository at this point in the history
Fix #3935
  • Loading branch information
justinsb committed Dec 1, 2017
1 parent 398c4ce commit 1993d37
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
38 changes: 31 additions & 7 deletions cmd/kops/replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}

Expand All @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions pkg/client/simple/vfsclientset/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 1993d37

Please sign in to comment.