All commands must declare Unstructured or Internal

Callers must take a dependency on one or the other set of conversions
and default client behavior. Future changes may add a Versioned() type,
but this is an accurate reflection of current code state.
pull/6/head
Clayton Coleman 2017-11-15 01:10:30 -05:00
parent 04ab96d2bd
commit 8f4b6c8771
No known key found for this signature in database
GPG Key ID: 3D16906B4F1C5CB3
34 changed files with 135 additions and 39 deletions

View File

@ -144,6 +144,7 @@ func (p *AttachOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, argsIn [
}
builder := f.NewBuilder().
Internal().
NamespaceParam(namespace).DefaultNamespace()
switch len(argsIn) {

View File

@ -94,11 +94,13 @@ func (o *ReconcileOptions) Complete(cmd *cobra.Command, f cmdutil.Factory, args
}
r := f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(namespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).
Flatten().
Do()
if err := r.Err(); err != nil {
return err
}

View File

@ -91,6 +91,7 @@ func RunAutoscale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s
}
r := f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(namespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).

View File

@ -168,6 +168,7 @@ func (options *CertificateOptions) modifyCertificateCondition(f cmdutil.Factory,
return err
}
r := f.NewBuilder().
Internal().
ContinueOnError().
FilenameParam(false, &options.FilenameOptions).
ResourceNames("certificatesigningrequest", options.csrNames...).

View File

@ -73,6 +73,7 @@ func RunClusterInfo(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error
// TODO use generalized labels once they are implemented (#341)
b := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
LabelSelectorParam("kubernetes.io/cluster-service=true").
ResourceTypeOrNameArgs(false, []string{"services"}...).

View File

@ -128,7 +128,9 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C
}
// build the builder
o.builder = f.NewBuilder().LocalParam(o.local)
o.builder = f.NewBuilder().
Internal().
LocalParam(o.local)
if !o.local {
schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"))
if err != nil {

View File

@ -444,13 +444,15 @@ func TestClusterRoleValidate(t *testing.T) {
}
for name, test := range tests {
test.clusterRoleOptions.Mapper, _ = f.Object()
err := test.clusterRoleOptions.Validate()
if test.expectErr && err == nil {
t.Errorf("%s: expect error happens, but validate passes.", name)
}
if !test.expectErr && err != nil {
t.Errorf("%s: unexpected error: %v", name, err)
}
t.Run(name, func(t *testing.T) {
test.clusterRoleOptions.Mapper, _ = f.Object()
err := test.clusterRoleOptions.Validate()
if test.expectErr && err == nil {
t.Errorf("%s: expect error happens, but validate passes.", name)
}
if !test.expectErr && err != nil {
t.Errorf("%s: unexpected error: %v", name, err)
}
})
}
}

View File

@ -25,6 +25,7 @@ import (
rbac "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/client-go/rest/fake"
cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
)
@ -143,19 +144,21 @@ func TestCreateRole(t *testing.T) {
}
for name, test := range tests {
buf := bytes.NewBuffer([]byte{})
cmd := NewCmdCreateRole(f, buf)
cmd.Flags().Set("dry-run", "true")
cmd.Flags().Set("output", "object")
cmd.Flags().Set("verb", test.verbs)
cmd.Flags().Set("resource", test.resources)
if test.resourceNames != "" {
cmd.Flags().Set("resource-name", test.resourceNames)
}
cmd.Run(cmd, []string{roleName})
if !reflect.DeepEqual(test.expectedRole, printer.CachedRole) {
t.Errorf("%s:\nexpected:\n%#v\nsaw:\n%#v", name, test.expectedRole, printer.CachedRole)
}
t.Run(name, func(t *testing.T) {
buf := bytes.NewBuffer([]byte{})
cmd := NewCmdCreateRole(f, buf)
cmd.Flags().Set("dry-run", "true")
cmd.Flags().Set("output", "object")
cmd.Flags().Set("verb", test.verbs)
cmd.Flags().Set("resource", test.resources)
if test.resourceNames != "" {
cmd.Flags().Set("resource-name", test.resourceNames)
}
cmd.Run(cmd, []string{roleName})
if !reflect.DeepEqual(test.expectedRole, printer.CachedRole) {
t.Errorf("%s", diff.ObjectReflectDiff(test.expectedRole, printer.CachedRole))
}
})
}
}

View File

@ -176,7 +176,8 @@ func RunDescribe(f cmdutil.Factory, out, cmdErr io.Writer, cmd *cobra.Command, a
}
func DescribeMatchingResources(f cmdutil.Factory, namespace, rsrc, prefix string, describerSettings *printers.DescriberSettings, out io.Writer, originalError error) error {
r := f.NewBuilder().Unstructured().
r := f.NewBuilder().
Unstructured().
NamespaceParam(namespace).DefaultNamespace().
ResourceTypeOrNameArgs(true, rsrc).
SingleResourceType().

View File

@ -237,6 +237,7 @@ func (o *DrainOptions) SetupDrain(cmd *cobra.Command, args []string) error {
}
builder := o.Factory.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
ResourceNames("nodes", args...).
SingleResourceType().

View File

@ -127,6 +127,7 @@ func RunExpose(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri
mapper, typer := f.Object()
r := f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(namespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).

View File

@ -193,6 +193,7 @@ func (o *LogsOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.Comm
if o.Object == nil {
builder := f.NewBuilder().
Internal().
NamespaceParam(o.Namespace).DefaultNamespace().
SingleResourceType()
if o.ResourceArg != "" {

View File

@ -188,7 +188,8 @@ func forceReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s
}
}
r := f.NewBuilder().Unstructured().
r := f.NewBuilder().
Unstructured().
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).

View File

@ -201,6 +201,7 @@ func RunRollingUpdate(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args
}
request := f.NewBuilder().
Internal().
Schema(schema).
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, &resource.FilenameOptions{Recursive: false, Filenames: []string{filename}}).

View File

@ -80,6 +80,7 @@ func RunHistory(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []str
}
r := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).
ResourceTypeOrNameArgs(true, args...).

View File

@ -114,6 +114,7 @@ func (o *PauseConfig) CompletePause(f cmdutil.Factory, cmd *cobra.Command, out i
}
r := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, &o.FilenameOptions).
ResourceTypeOrNameArgs(true, args...).

View File

@ -112,6 +112,7 @@ func (o *ResumeConfig) CompleteResume(f cmdutil.Factory, cmd *cobra.Command, out
}
r := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, &o.FilenameOptions).
ResourceTypeOrNameArgs(true, args...).

View File

@ -83,6 +83,7 @@ func RunStatus(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []stri
}
r := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).
ResourceTypeOrNameArgs(true, args...).

View File

@ -113,6 +113,7 @@ func (o *UndoOptions) CompleteUndo(f cmdutil.Factory, cmd *cobra.Command, out io
}
r := f.NewBuilder().
Internal().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, &o.FilenameOptions).
ResourceTypeOrNameArgs(true, args...).

View File

@ -354,6 +354,7 @@ func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *c
return err
}
r := f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(namespace).DefaultNamespace().
ResourceNames(obj.Mapping.Resource, name).

View File

@ -105,6 +105,7 @@ func RunScale(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
mapper, _ := f.Object()
r := f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().
FilenameParam(enforceNamespace, options).

View File

@ -236,6 +236,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
if len(o.From) != 0 {
b := f.NewBuilder().
Internal().
LocalParam(o.Local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().
@ -303,6 +304,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
}
b := f.NewBuilder().
Internal().
LocalParam(o.Local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -143,6 +143,7 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st
includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
builder := f.NewBuilder().
Internal().
LocalParam(o.Local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -145,6 +145,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args
includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
builder := f.NewBuilder().
Internal().
LocalParam(o.Local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -129,6 +129,7 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [
includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
o.builder = f.NewBuilder().
Internal().
LocalParam(o.local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -130,6 +130,7 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com
resources := args[:len(args)-1]
includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
builder := f.NewBuilder().
Internal().
LocalParam(saConfig.local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -126,6 +126,7 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []
includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
builder := f.NewBuilder().
Internal().
LocalParam(o.Local).
ContinueOnError().
NamespaceParam(cmdNamespace).DefaultNamespace().

View File

@ -150,6 +150,7 @@ func (o *TaintOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.Com
return cmdutil.UsageErrorf(cmd, err.Error())
}
o.builder = f.NewBuilder().
Internal().
ContinueOnError().
NamespaceParam(namespace).DefaultNamespace()
if o.selector != "" {

View File

@ -288,7 +288,7 @@ func (f *FakeFactory) Object() (meta.RESTMapper, runtime.ObjectTyper) {
}
groupResources := testDynamicResources()
mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructuredConversion(legacyscheme.Registry.InterfacesFor))
typer := discovery.NewUnstructuredObjectTyper(groupResources)
typer := discovery.NewUnstructuredObjectTyper(groupResources, legacyscheme.Scheme)
fakeDs := &fakeCachedDiscoveryClient{}
expander := cmdutil.NewShortcutExpander(mapper, fakeDs)
@ -624,8 +624,17 @@ func (f *fakeAPIFactory) Object() (meta.RESTMapper, runtime.ObjectTyper) {
}
}),
)
// for backwards compatibility with existing tests, allow rest mappings from the scheme to show up
// TODO: make this opt-in?
mapper = meta.FirstHitRESTMapper{
MultiRESTMapper: meta.MultiRESTMapper{
mapper,
legacyscheme.Registry.RESTMapper(),
},
}
typer := discovery.NewUnstructuredObjectTyper(groupResources)
// TODO: should probably be the external scheme
typer := discovery.NewUnstructuredObjectTyper(groupResources, legacyscheme.Scheme)
fakeDs := &fakeCachedDiscoveryClient{}
expander := cmdutil.NewShortcutExpander(mapper, fakeDs)
return expander, typer
@ -925,6 +934,8 @@ func testDynamicResources() []*discovery.APIGroupResources {
{Name: "secrets", Namespaced: true, Kind: "Secret"},
{Name: "configmaps", Namespaced: true, Kind: "ConfigMap"},
{Name: "namespacedtype", Namespaced: true, Kind: "NamespacedType"},
{Name: "namespaces", Namespaced: false, Kind: "Namespace"},
{Name: "resourcequotas", Namespaced: true, Kind: "ResourceQuota"},
},
},
},
@ -939,6 +950,7 @@ func testDynamicResources() []*discovery.APIGroupResources {
VersionedResources: map[string][]metav1.APIResource{
"v1beta1": {
{Name: "deployments", Namespaced: true, Kind: "Deployment"},
{Name: "replicasets", Namespaced: true, Kind: "ReplicaSet"},
},
},
},
@ -955,12 +967,14 @@ func testDynamicResources() []*discovery.APIGroupResources {
VersionedResources: map[string][]metav1.APIResource{
"v1beta1": {
{Name: "deployments", Namespaced: true, Kind: "Deployment"},
{Name: "replicasets", Namespaced: true, Kind: "ReplicaSet"},
},
"v1beta2": {
{Name: "deployments", Namespaced: true, Kind: "Deployment"},
},
"v1": {
{Name: "deployments", Namespaced: true, Kind: "Deployment"},
{Name: "replicasets", Namespaced: true, Kind: "ReplicaSet"},
},
},
},
@ -1001,6 +1015,24 @@ func testDynamicResources() []*discovery.APIGroupResources {
},
},
},
{
Group: metav1.APIGroup{
Name: "rbac.authorization.k8s.io",
Versions: []metav1.GroupVersionForDiscovery{
{Version: "v1beta1"},
{Version: "v1"},
},
PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v1"},
},
VersionedResources: map[string][]metav1.APIResource{
"v1": {
{Name: "clusterroles", Namespaced: false, Kind: "ClusterRole"},
},
"v1beta1": {
{Name: "clusterrolebindings", Namespaced: false, Kind: "ClusterRoleBinding"},
},
},
},
{
Group: metav1.APIGroup{
Name: "company.com",

View File

@ -9,7 +9,6 @@ go_library(
srcs = [
"cached_discovery.go",
"clientcache.go",
"discovery.go",
"factory.go",
"factory_builder.go",
"factory_client_access.go",

View File

@ -108,7 +108,8 @@ func (o *EditOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args []
return err
}
mapper, _ := f.Object()
b := f.NewBuilder().Unstructured()
b := f.NewBuilder().
Unstructured()
if o.EditMode == NormalEditMode || o.EditMode == ApplyEditMode {
// when do normal edit or apply edit we need to always retrieve the latest resource from server
b = b.ResourceTypeOrNameArgs(true, args...).Latest()

View File

@ -98,13 +98,13 @@ func (f *ring1Factory) objectLoader() (meta.RESTMapper, runtime.ObjectTyper, err
interfaces := meta.InterfacesForUnstructuredConversion(legacyscheme.Registry.InterfacesFor)
mapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.VersionInterfacesFunc(interfaces))
// TODO: should this also indicate it recognizes typed objects?
typer := discovery.NewUnstructuredObjectTyper(groupResources)
typer := discovery.NewUnstructuredObjectTyper(groupResources, legacyscheme.Scheme)
expander := NewShortcutExpander(mapper, discoveryClient)
return expander, typer, err
}
func (f *ring1Factory) Object() (meta.RESTMapper, runtime.ObjectTyper) {
return NewLazyObjectLoader(f.objectLoader)
return meta.NewLazyObjectLoader(f.objectLoader)
}
func (f *ring1Factory) CategoryExpander() categories.CategoryExpander {

View File

@ -43,10 +43,13 @@ const defaultHttpGetAttempts int = 3
// from the command line and converting them to a list of resources to iterate
// over using the Visitor interface.
type Builder struct {
mapper *Mapper
unstructured *Mapper
categoryExpander categories.CategoryExpander
// mapper is set explicitly by resource builders
mapper *Mapper
internal *Mapper
unstructured *Mapper
errs []error
paths []Visitor
@ -116,10 +119,12 @@ type resourceTuple struct {
Name string
}
// NewBuilder creates a builder that operates on generic objects.
func NewBuilder(mapper, unstructured *Mapper, categoryExpander categories.CategoryExpander) *Builder {
// NewBuilder creates a builder that operates on generic objects. At least one of
// internal or unstructured must be specified.
// TODO: Add versioned client (although versioned is still lossy)
func NewBuilder(internal, unstructured *Mapper, categoryExpander categories.CategoryExpander) *Builder {
return &Builder{
mapper: mapper,
internal: internal,
unstructured: unstructured,
categoryExpander: categoryExpander,
requireObject: true,
@ -167,16 +172,41 @@ func (b *Builder) FilenameParam(enforceNamespace bool, filenameOptions *Filename
}
// Unstructured updates the builder so that it will request and send unstructured
// objects by default. Calling this method resets Local().
// objects. Unstructured objects preserve all fields sent by the server in a map format
// based on the object's JSON structure which means no data is lost when the client
// reads and then writes an object. Use this mode in preference to Internal unless you
// are working with Go types directly.
func (b *Builder) Unstructured() *Builder {
if b.unstructured == nil {
b.errs = append(b.errs, fmt.Errorf("no unstructured builder provided"))
b.errs = append(b.errs, fmt.Errorf("no unstructured mapper provided"))
return b
}
if b.mapper != nil {
b.errs = append(b.errs, fmt.Errorf("another mapper was already selected, cannot use unstructured types"))
return b
}
b.mapper = b.unstructured
return b
}
// Internal updates the builder so that it will convert objects off the wire
// into the internal form if necessary. Using internal types is lossy - fields added
// to the server will not be seen by the client code and may result in failure. Only
// use this mode when working offline, or when generating patches to send to the server.
// Use Unstructured if you are reading an object and performing a POST or PUT.
func (b *Builder) Internal() *Builder {
if b.internal == nil {
b.errs = append(b.errs, fmt.Errorf("no internal mapper provided"))
return b
}
if b.mapper != nil {
b.errs = append(b.errs, fmt.Errorf("another mapper was already selected, cannot use internal types"))
return b
}
b.mapper = b.internal
return b
}
// LocalParam calls Local() if local is true.
func (b *Builder) LocalParam(local bool) *Builder {
if local {

View File

@ -277,7 +277,7 @@ func newDefaultBuilderWith(client ClientMapper) *Builder {
},
nil,
categories.LegacyCategoryExpander,
)
).Internal()
}
func TestPathBuilderAndVersionedObjectNotDefaulted(t *testing.T) {
@ -628,7 +628,7 @@ func TestMultipleResourceByTheSameName(t *testing.T) {
func TestRequestModifier(t *testing.T) {
var got *rest.Request
b := NewBuilder(restmapper, categories.LegacyCategoryExpander, scheme.Scheme, fakeClientWith("test", t, nil), corev1Codec).
b := newDefaultBuilderWith(fakeClientWith("test", t, nil)).
NamespaceParam("foo").
TransformRequests(func(req *rest.Request) {
got = req