From 87b2a3129b57adf8872c4cc0ddc43646c23fe9b3 Mon Sep 17 00:00:00 2001 From: Ted Yu Date: Mon, 12 Aug 2019 13:55:33 -0700 Subject: [PATCH] Propagate error from NewREST --- pkg/master/master.go | 29 ++++-- pkg/master/master_test.go | 5 +- .../storage/storage.go | 6 +- .../rest/storage_apiserver.go | 44 ++++++--- .../storage/storage.go | 6 +- .../controllerrevision/storage/storage.go | 6 +- .../storage/storage_test.go | 5 +- .../apps/daemonset/storage/storage.go | 6 +- .../apps/daemonset/storage/storage_test.go | 5 +- .../apps/deployment/storage/storage.go | 15 +-- .../apps/deployment/storage/storage_test.go | 5 +- .../apps/replicaset/storage/storage.go | 15 +-- .../apps/replicaset/storage/storage_test.go | 5 +- pkg/registry/apps/rest/storage_apps.go | 99 ++++++++++++++----- .../apps/statefulset/storage/storage.go | 15 +-- .../apps/statefulset/storage/storage_test.go | 5 +- .../auditsink/storage/storage.go | 6 +- .../rest/storage_auditregistration.go | 16 +-- .../rest/storage_authentication.go | 4 +- .../rest/storage_authorization.go | 6 +- .../storage/storage.go | 6 +- .../storage/storage_test.go | 5 +- .../autoscaling/rest/storage_autoscaling.go | 49 ++++++--- pkg/registry/batch/cronjob/storage/storage.go | 6 +- .../batch/cronjob/storage/storage_test.go | 5 +- pkg/registry/batch/job/storage/storage.go | 15 +-- .../batch/job/storage/storage_test.go | 5 +- pkg/registry/batch/rest/storage_batch.go | 49 ++++++--- .../certificates/storage/storage.go | 6 +- .../certificates/rest/storage_certificates.go | 19 ++-- .../coordination/lease/storage/storage.go | 6 +- .../coordination/rest/storage_coordination.go | 34 +++++-- .../core/configmap/storage/storage.go | 6 +- .../core/configmap/storage/storage_test.go | 6 +- pkg/registry/core/endpoint/storage/storage.go | 6 +- .../core/endpoint/storage/storage_test.go | 6 +- pkg/registry/core/event/storage/storage.go | 8 +- .../core/event/storage/storage_test.go | 6 +- .../core/limitrange/storage/storage.go | 6 +- .../core/limitrange/storage/storage_test.go | 6 +- .../core/namespace/storage/storage.go | 6 +- .../core/namespace/storage/storage_test.go | 15 ++- .../core/persistentvolume/storage/storage.go | 6 +- .../persistentvolume/storage/storage_test.go | 5 +- .../persistentvolumeclaim/storage/storage.go | 6 +- .../storage/storage_test.go | 5 +- .../core/pod/storage/eviction_test.go | 5 +- pkg/registry/core/pod/storage/storage.go | 6 +- pkg/registry/core/pod/storage/storage_test.go | 10 +- .../core/podtemplate/storage/storage.go | 6 +- .../core/podtemplate/storage/storage_test.go | 6 +- .../replicationcontroller/storage/storage.go | 15 +-- .../storage/storage_test.go | 5 +- .../core/resourcequota/storage/storage.go | 6 +- .../resourcequota/storage/storage_test.go | 5 +- pkg/registry/core/rest/storage_core.go | 72 +++++++++++--- pkg/registry/core/secret/storage/storage.go | 6 +- .../core/secret/storage/storage_test.go | 6 +- .../core/service/storage/rest_test.go | 10 +- pkg/registry/core/service/storage/storage.go | 6 +- .../core/service/storage/storage_test.go | 5 +- .../core/serviceaccount/storage/storage.go | 6 +- .../serviceaccount/storage/storage_test.go | 6 +- pkg/registry/events/rest/storage_events.go | 19 ++-- .../extensions/controller/storage/storage.go | 9 +- .../controller/storage/storage_test.go | 6 +- .../extensions/rest/storage_extensions.go | 49 ++++++--- .../networking/ingress/storage/storage.go | 6 +- .../ingress/storage/storage_test.go | 5 +- .../networkpolicy/storage/storage.go | 6 +- .../networkpolicy/storage/storage_test.go | 6 +- .../networking/rest/storage_settings.go | 34 +++++-- pkg/registry/node/rest/runtime_class.go | 34 +++++-- .../node/runtimeclass/storage/storage.go | 6 +- .../poddisruptionbudget/storage/storage.go | 6 +- .../storage/storage_test.go | 5 +- .../podsecuritypolicy/storage/storage.go | 6 +- .../podsecuritypolicy/storage/storage_test.go | 6 +- pkg/registry/policy/rest/storage_policy.go | 25 +++-- .../rbac/clusterrole/storage/storage.go | 6 +- .../clusterrolebinding/storage/storage.go | 6 +- pkg/registry/rbac/rest/storage_rbac.go | 46 ++++++--- pkg/registry/rbac/role/storage/storage.go | 6 +- .../rbac/rolebinding/storage/storage.go | 6 +- .../priorityclass/storage/storage.go | 7 +- .../priorityclass/storage/storage_test.go | 6 +- .../scheduling/rest/storage_scheduling.go | 56 +++++++---- .../settings/podpreset/storage/storage.go | 6 +- .../podpreset/storage/storage_test.go | 6 +- .../settings/rest/storage_settings.go | 19 ++-- .../storage/csidriver/storage/storage.go | 6 +- .../storage/csidriver/storage/storage_test.go | 5 +- .../storage/csinode/storage/storage.go | 6 +- .../storage/csinode/storage/storage_test.go | 5 +- pkg/registry/storage/rest/storage_storage.go | 69 +++++++++---- .../storage/storageclass/storage/storage.go | 6 +- .../storageclass/storage/storage_test.go | 5 +- .../volumeattachment/storage/storage.go | 6 +- .../volumeattachment/storage/storage_test.go | 5 +- test/integration/auth/rbac_test.go | 32 ++++-- 100 files changed, 917 insertions(+), 408 deletions(-) diff --git a/pkg/master/master.go b/pkg/master/master.go index 48b74bb6104..578452c0889 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -329,7 +329,9 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) ServiceAccountMaxExpiration: c.ExtraConfig.ServiceAccountMaxExpiration, APIAudiences: c.GenericConfig.Authentication.APIAudiences, } - m.InstallLegacyAPI(&c, c.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider) + if err := m.InstallLegacyAPI(&c, c.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider); err != nil { + return nil, err + } } // The order here is preserved in discovery. @@ -361,7 +363,9 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) admissionregistrationrest.RESTStorageProvider{}, eventsrest.RESTStorageProvider{TTL: c.ExtraConfig.EventTTL}, } - m.InstallAPIs(c.ExtraConfig.APIResourceConfigSource, c.GenericConfig.RESTOptionsGetter, restStorageProviders...) + if err := m.InstallAPIs(c.ExtraConfig.APIResourceConfigSource, c.GenericConfig.RESTOptionsGetter, restStorageProviders...); err != nil { + return nil, err + } if c.ExtraConfig.Tunneler != nil { m.installTunneler(c.ExtraConfig.Tunneler, corev1client.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig).Nodes()) @@ -372,10 +376,10 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) return m, nil } -func (m *Master) InstallLegacyAPI(c *completedConfig, restOptionsGetter generic.RESTOptionsGetter, legacyRESTStorageProvider corerest.LegacyRESTStorageProvider) { +func (m *Master) InstallLegacyAPI(c *completedConfig, restOptionsGetter generic.RESTOptionsGetter, legacyRESTStorageProvider corerest.LegacyRESTStorageProvider) error { legacyRESTStorage, apiGroupInfo, err := legacyRESTStorageProvider.NewLegacyRESTStorage(restOptionsGetter) if err != nil { - klog.Fatalf("Error building core storage: %v", err) + return fmt.Errorf("Error building core storage: %v", err) } controllerName := "bootstrap-controller" @@ -385,8 +389,9 @@ func (m *Master) InstallLegacyAPI(c *completedConfig, restOptionsGetter generic. m.GenericAPIServer.AddPreShutdownHookOrDie(controllerName, bootstrapController.PreShutdownHook) if err := m.GenericAPIServer.InstallLegacyAPIGroup(genericapiserver.DefaultLegacyAPIPrefix, &apiGroupInfo); err != nil { - klog.Fatalf("Error in registering group versions: %v", err) + return fmt.Errorf("Error in registering group versions: %v", err) } + return nil } func (m *Master) installTunneler(nodeTunneler tunneler.Tunneler, nodeClient corev1client.NodeInterface) { @@ -405,11 +410,11 @@ func (m *Master) installTunneler(nodeTunneler tunneler.Tunneler, nodeClient core // RESTStorageProvider is a factory type for REST storage. type RESTStorageProvider interface { GroupName() string - NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) + NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) } // InstallAPIs will install the APIs for the restStorageProviders if they are enabled. -func (m *Master) InstallAPIs(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter, restStorageProviders ...RESTStorageProvider) { +func (m *Master) InstallAPIs(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter, restStorageProviders ...RESTStorageProvider) error { apiGroupsInfo := []*genericapiserver.APIGroupInfo{} for _, restStorageBuilder := range restStorageProviders { @@ -418,9 +423,12 @@ func (m *Master) InstallAPIs(apiResourceConfigSource serverstorage.APIResourceCo klog.V(1).Infof("Skipping disabled API group %q.", groupName) continue } - apiGroupInfo, enabled := restStorageBuilder.NewRESTStorage(apiResourceConfigSource, restOptionsGetter) + apiGroupInfo, enabled, err := restStorageBuilder.NewRESTStorage(apiResourceConfigSource, restOptionsGetter) + if err != nil { + return fmt.Errorf("problem initializing API group %q : %v.", groupName, err) + } if !enabled { - klog.Warningf("Problem initializing API group %q, skipping.", groupName) + klog.Warningf("API group %q is not enabled, skipping.", groupName) continue } klog.V(1).Infof("Enabling API group %q.", groupName) @@ -437,8 +445,9 @@ func (m *Master) InstallAPIs(apiResourceConfigSource serverstorage.APIResourceCo } if err := m.GenericAPIServer.InstallAPIGroups(apiGroupsInfo...); err != nil { - klog.Fatalf("Error in registering group versions: %v", err) + return fmt.Errorf("Error in registering group versions: %v", err) } + return nil } type nodeAddressProvider struct { diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 1a920eab8e3..383d62a04e1 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -182,7 +182,10 @@ func TestCertificatesRestStorageStrategies(t *testing.T) { defer etcdserver.Terminate(t) certStorageProvider := certificatesrest.RESTStorageProvider{} - apiGroupInfo, _ := certStorageProvider.NewRESTStorage(masterCfg.ExtraConfig.APIResourceConfigSource, masterCfg.GenericConfig.RESTOptionsGetter) + apiGroupInfo, _, err := certStorageProvider.NewRESTStorage(masterCfg.ExtraConfig.APIResourceConfigSource, masterCfg.GenericConfig.RESTOptionsGetter) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } exceptions := registrytest.StrategyExceptions{ HasExportStrategy: []string{ diff --git a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/storage/storage.go b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/storage/storage.go index 1b6b03097d8..28a57c0f1fe 100644 --- a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/storage/storage.go +++ b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against pod disruption budgets. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &admissionregistration.MutatingWebhookConfiguration{} }, NewListFunc: func() runtime.Object { return &admissionregistration.MutatingWebhookConfigurationList{} }, @@ -45,7 +45,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/admissionregistration/rest/storage_apiserver.go b/pkg/registry/admissionregistration/rest/storage_apiserver.go index f0ea9f95028..565d72e4933 100644 --- a/pkg/registry/admissionregistration/rest/storage_apiserver.go +++ b/pkg/registry/admissionregistration/rest/storage_apiserver.go @@ -31,44 +31,64 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(admissionregistration.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(admissionregistrationv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[admissionregistrationv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[admissionregistrationv1beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(admissionregistrationv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[admissionregistrationv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[admissionregistrationv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // validatingwebhookconfigurations - validatingStorage := validatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + validatingStorage, err := validatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["validatingwebhookconfigurations"] = validatingStorage // mutatingwebhookconfigurations - mutatingStorage := mutatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + mutatingStorage, err := mutatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["mutatingwebhookconfigurations"] = mutatingStorage - return storage + return storage, err } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // validatingwebhookconfigurations - validatingStorage := validatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + validatingStorage, err := validatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["validatingwebhookconfigurations"] = validatingStorage // mutatingwebhookconfigurations - mutatingStorage := mutatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + mutatingStorage, err := mutatingwebhookconfigurationstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["mutatingwebhookconfigurations"] = mutatingStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/admissionregistration/validatingwebhookconfiguration/storage/storage.go b/pkg/registry/admissionregistration/validatingwebhookconfiguration/storage/storage.go index f0e1baf8604..cecb6cbb833 100644 --- a/pkg/registry/admissionregistration/validatingwebhookconfiguration/storage/storage.go +++ b/pkg/registry/admissionregistration/validatingwebhookconfiguration/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against pod disruption budgets. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &admissionregistration.ValidatingWebhookConfiguration{} }, NewListFunc: func() runtime.Object { return &admissionregistration.ValidatingWebhookConfigurationList{} }, @@ -45,7 +45,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/apps/controllerrevision/storage/storage.go b/pkg/registry/apps/controllerrevision/storage/storage.go index 92085b59387..f00f1b80992 100644 --- a/pkg/registry/apps/controllerrevision/storage/storage.go +++ b/pkg/registry/apps/controllerrevision/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work with ControllerRevision objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &apps.ControllerRevision{} }, NewListFunc: func() runtime.Object { return &apps.ControllerRevisionList{} }, @@ -47,7 +47,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/apps/controllerrevision/storage/storage_test.go b/pkg/registry/apps/controllerrevision/storage/storage_test.go index 415bbdc7b60..7a531226cd5 100644 --- a/pkg/registry/apps/controllerrevision/storage/storage_test.go +++ b/pkg/registry/apps/controllerrevision/storage/storage_test.go @@ -160,7 +160,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "controllerrevisions"} - storage := NewREST(restOptions) + storage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storage, server } diff --git a/pkg/registry/apps/daemonset/storage/storage.go b/pkg/registry/apps/daemonset/storage/storage.go index d13d6f9d5ad..e824f200143 100644 --- a/pkg/registry/apps/daemonset/storage/storage.go +++ b/pkg/registry/apps/daemonset/storage/storage.go @@ -38,7 +38,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against DaemonSets. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &apps.DaemonSet{} }, NewListFunc: func() runtime.Object { return &apps.DaemonSetList{} }, @@ -52,13 +52,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = daemonset.StatusStrategy - return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore} + return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/apps/daemonset/storage/storage_test.go b/pkg/registry/apps/daemonset/storage/storage_test.go index d969a2d774f..eaf15ffcdae 100644 --- a/pkg/registry/apps/daemonset/storage/storage_test.go +++ b/pkg/registry/apps/daemonset/storage/storage_test.go @@ -39,7 +39,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "daemonsets", } - daemonSetStorage, statusStorage := NewREST(restOptions) + daemonSetStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return daemonSetStorage, statusStorage, server } diff --git a/pkg/registry/apps/deployment/storage/storage.go b/pkg/registry/apps/deployment/storage/storage.go index 672e50e9560..8bff82cf9f0 100644 --- a/pkg/registry/apps/deployment/storage/storage.go +++ b/pkg/registry/apps/deployment/storage/storage.go @@ -53,15 +53,18 @@ type DeploymentStorage struct { Rollback *RollbackREST } -func NewStorage(optsGetter generic.RESTOptionsGetter) DeploymentStorage { - deploymentRest, deploymentStatusRest, deploymentRollbackRest := NewREST(optsGetter) +func NewStorage(optsGetter generic.RESTOptionsGetter) (DeploymentStorage, error) { + deploymentRest, deploymentStatusRest, deploymentRollbackRest, err := NewREST(optsGetter) + if err != nil { + return DeploymentStorage{}, err + } return DeploymentStorage{ Deployment: deploymentRest, Status: deploymentStatusRest, Scale: &ScaleREST{store: deploymentRest.Store}, Rollback: deploymentRollbackRest, - } + }, nil } type REST struct { @@ -70,7 +73,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against deployments. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *RollbackREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *RollbackREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &apps.Deployment{} }, NewListFunc: func() runtime.Object { return &apps.DeploymentList{} }, @@ -84,12 +87,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Rollbac } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, nil, err } statusStore := *store statusStore.UpdateStrategy = deployment.StatusStrategy - return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, &RollbackREST{store: store} + return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, &RollbackREST{store: store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/apps/deployment/storage/storage_test.go b/pkg/registry/apps/deployment/storage/storage_test.go index 036e8b0ce84..a1acf97c3bc 100644 --- a/pkg/registry/apps/deployment/storage/storage_test.go +++ b/pkg/registry/apps/deployment/storage/storage_test.go @@ -47,7 +47,10 @@ const defaultReplicas = 100 func newStorage(t *testing.T) (*DeploymentStorage, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, apps.GroupName) restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "deployments"} - deploymentStorage := NewStorage(restOptions) + deploymentStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return &deploymentStorage, server } diff --git a/pkg/registry/apps/replicaset/storage/storage.go b/pkg/registry/apps/replicaset/storage/storage.go index 9fed032d26b..32be92d821b 100644 --- a/pkg/registry/apps/replicaset/storage/storage.go +++ b/pkg/registry/apps/replicaset/storage/storage.go @@ -49,14 +49,17 @@ type ReplicaSetStorage struct { Scale *ScaleREST } -func NewStorage(optsGetter generic.RESTOptionsGetter) ReplicaSetStorage { - replicaSetRest, replicaSetStatusRest := NewREST(optsGetter) +func NewStorage(optsGetter generic.RESTOptionsGetter) (ReplicaSetStorage, error) { + replicaSetRest, replicaSetStatusRest, err := NewREST(optsGetter) + if err != nil { + return ReplicaSetStorage{}, err + } return ReplicaSetStorage{ ReplicaSet: replicaSetRest, Status: replicaSetStatusRest, Scale: &ScaleREST{store: replicaSetRest.Store}, - } + }, nil } type REST struct { @@ -65,7 +68,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against ReplicaSet. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &apps.ReplicaSet{} }, NewListFunc: func() runtime.Object { return &apps.ReplicaSetList{} }, @@ -80,13 +83,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: replicaset.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = replicaset.StatusStrategy - return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore} + return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/apps/replicaset/storage/storage_test.go b/pkg/registry/apps/replicaset/storage/storage_test.go index 1fa0b868b6f..ca4b10c2542 100644 --- a/pkg/registry/apps/replicaset/storage/storage_test.go +++ b/pkg/registry/apps/replicaset/storage/storage_test.go @@ -42,7 +42,10 @@ const defaultReplicas = 100 func newStorage(t *testing.T) (*ReplicaSetStorage, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, "extensions") restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "replicasets"} - replicaSetStorage := NewStorage(restOptions) + replicaSetStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return &replicaSetStorage, server } diff --git a/pkg/registry/apps/rest/storage_apps.go b/pkg/registry/apps/rest/storage_apps.go index b8e683ee9a1..866d0fe6603 100644 --- a/pkg/registry/apps/rest/storage_apps.go +++ b/pkg/registry/apps/rest/storage_apps.go @@ -35,111 +35,162 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(apps.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(appsapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[appsapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[appsapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(appsapiv1beta2.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[appsapiv1beta2.SchemeGroupVersion.Version] = p.v1beta2Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta2Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[appsapiv1beta2.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(appsapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[appsapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[appsapiv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // deployments - deploymentStorage := deploymentstore.NewStorage(restOptionsGetter) + deploymentStorage, err := deploymentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["deployments"] = deploymentStorage.Deployment storage["deployments/status"] = deploymentStorage.Status storage["deployments/rollback"] = deploymentStorage.Rollback storage["deployments/scale"] = deploymentStorage.Scale // statefulsets - statefulSetStorage := statefulsetstore.NewStorage(restOptionsGetter) + statefulSetStorage, err := statefulsetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["statefulsets"] = statefulSetStorage.StatefulSet storage["statefulsets/status"] = statefulSetStorage.Status storage["statefulsets/scale"] = statefulSetStorage.Scale // controllerrevisions - historyStorage := controllerrevisionsstore.NewREST(restOptionsGetter) + historyStorage, err := controllerrevisionsstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["controllerrevisions"] = historyStorage - return storage + return storage, nil } -func (p RESTStorageProvider) v1beta2Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta2Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // deployments - deploymentStorage := deploymentstore.NewStorage(restOptionsGetter) + deploymentStorage, err := deploymentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["deployments"] = deploymentStorage.Deployment storage["deployments/status"] = deploymentStorage.Status storage["deployments/scale"] = deploymentStorage.Scale // statefulsets - statefulSetStorage := statefulsetstore.NewStorage(restOptionsGetter) + statefulSetStorage, err := statefulsetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["statefulsets"] = statefulSetStorage.StatefulSet storage["statefulsets/status"] = statefulSetStorage.Status storage["statefulsets/scale"] = statefulSetStorage.Scale // daemonsets - daemonSetStorage, daemonSetStatusStorage := daemonsetstore.NewREST(restOptionsGetter) + daemonSetStorage, daemonSetStatusStorage, err := daemonsetstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["daemonsets"] = daemonSetStorage storage["daemonsets/status"] = daemonSetStatusStorage // replicasets - replicaSetStorage := replicasetstore.NewStorage(restOptionsGetter) + replicaSetStorage, err := replicasetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["replicasets"] = replicaSetStorage.ReplicaSet storage["replicasets/status"] = replicaSetStorage.Status storage["replicasets/scale"] = replicaSetStorage.Scale // controllerrevisions - historyStorage := controllerrevisionsstore.NewREST(restOptionsGetter) + historyStorage, err := controllerrevisionsstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["controllerrevisions"] = historyStorage - return storage + return storage, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // deployments - deploymentStorage := deploymentstore.NewStorage(restOptionsGetter) + deploymentStorage, err := deploymentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["deployments"] = deploymentStorage.Deployment storage["deployments/status"] = deploymentStorage.Status storage["deployments/scale"] = deploymentStorage.Scale // statefulsets - statefulSetStorage := statefulsetstore.NewStorage(restOptionsGetter) + statefulSetStorage, err := statefulsetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["statefulsets"] = statefulSetStorage.StatefulSet storage["statefulsets/status"] = statefulSetStorage.Status storage["statefulsets/scale"] = statefulSetStorage.Scale // daemonsets - daemonSetStorage, daemonSetStatusStorage := daemonsetstore.NewREST(restOptionsGetter) + daemonSetStorage, daemonSetStatusStorage, err := daemonsetstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["daemonsets"] = daemonSetStorage storage["daemonsets/status"] = daemonSetStatusStorage // replicasets - replicaSetStorage := replicasetstore.NewStorage(restOptionsGetter) + replicaSetStorage, err := replicasetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["replicasets"] = replicaSetStorage.ReplicaSet storage["replicasets/status"] = replicaSetStorage.Status storage["replicasets/scale"] = replicaSetStorage.Scale // controllerrevisions - historyStorage := controllerrevisionsstore.NewREST(restOptionsGetter) + historyStorage, err := controllerrevisionsstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["controllerrevisions"] = historyStorage - return storage + return storage, nil } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/apps/statefulset/storage/storage.go b/pkg/registry/apps/statefulset/storage/storage.go index c7e325d90aa..65c6db82047 100644 --- a/pkg/registry/apps/statefulset/storage/storage.go +++ b/pkg/registry/apps/statefulset/storage/storage.go @@ -46,14 +46,17 @@ type StatefulSetStorage struct { Scale *ScaleREST } -func NewStorage(optsGetter generic.RESTOptionsGetter) StatefulSetStorage { - statefulSetRest, statefulSetStatusRest := NewREST(optsGetter) +func NewStorage(optsGetter generic.RESTOptionsGetter) (StatefulSetStorage, error) { + statefulSetRest, statefulSetStatusRest, err := NewREST(optsGetter) + if err != nil { + return StatefulSetStorage{}, err + } return StatefulSetStorage{ StatefulSet: statefulSetRest, Status: statefulSetStatusRest, Scale: &ScaleREST{store: statefulSetRest.Store}, - } + }, nil } // rest implements a RESTStorage for statefulsets against etcd @@ -62,7 +65,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against statefulsets. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &apps.StatefulSet{} }, NewListFunc: func() runtime.Object { return &apps.StatefulSetList{} }, @@ -76,12 +79,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = statefulset.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement CategoriesProvider diff --git a/pkg/registry/apps/statefulset/storage/storage_test.go b/pkg/registry/apps/statefulset/storage/storage_test.go index d4e1ad48873..4e392f41913 100644 --- a/pkg/registry/apps/statefulset/storage/storage_test.go +++ b/pkg/registry/apps/statefulset/storage/storage_test.go @@ -40,7 +40,10 @@ import ( func newStorage(t *testing.T) (StatefulSetStorage, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, apps.GroupName) restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "statefulsets"} - storage := NewStorage(restOptions) + storage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storage, server } diff --git a/pkg/registry/auditregistration/auditsink/storage/storage.go b/pkg/registry/auditregistration/auditsink/storage/storage.go index f0f071407a7..2ef0b5ee6b8 100644 --- a/pkg/registry/auditregistration/auditsink/storage/storage.go +++ b/pkg/registry/auditregistration/auditsink/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against audit sinks -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &auditregistration.AuditSink{} }, NewListFunc: func() runtime.Object { return &auditregistration.AuditSinkList{} }, @@ -45,7 +45,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/auditregistration/rest/storage_auditregistration.go b/pkg/registry/auditregistration/rest/storage_auditregistration.go index 4bc5e884969..4c218c41a90 100644 --- a/pkg/registry/auditregistration/rest/storage_auditregistration.go +++ b/pkg/registry/auditregistration/rest/storage_auditregistration.go @@ -30,21 +30,25 @@ import ( type RESTStorageProvider struct{} // NewRESTStorage returns a RESTStorageProvider -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(auditv1alpha1.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) if apiResourceConfigSource.VersionEnabled(auditv1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[auditv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[auditv1alpha1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} - s := auditstorage.NewREST(restOptionsGetter) + s, err := auditstorage.NewREST(restOptionsGetter) storage["auditsinks"] = s - return storage + return storage, err } // GroupName is the group name for the storage provider diff --git a/pkg/registry/authentication/rest/storage_authentication.go b/pkg/registry/authentication/rest/storage_authentication.go index 9dc3cf1a8e4..c99f9b3e8de 100644 --- a/pkg/registry/authentication/rest/storage_authentication.go +++ b/pkg/registry/authentication/rest/storage_authentication.go @@ -34,7 +34,7 @@ type RESTStorageProvider struct { APIAudiences authenticator.Audiences } -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { // TODO figure out how to make the swagger generation stable, while allowing this endpoint to be disabled. // if p.Authenticator == nil { // return genericapiserver.APIGroupInfo{}, false @@ -51,7 +51,7 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag apiGroupInfo.VersionedResourcesStorageMap[authenticationv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) } - return apiGroupInfo, true + return apiGroupInfo, true, nil } func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { diff --git a/pkg/registry/authorization/rest/storage_authorization.go b/pkg/registry/authorization/rest/storage_authorization.go index af79bf44134..00d8f5c96f6 100644 --- a/pkg/registry/authorization/rest/storage_authorization.go +++ b/pkg/registry/authorization/rest/storage_authorization.go @@ -37,9 +37,9 @@ type RESTStorageProvider struct { RuleResolver authorizer.RuleResolver } -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { if p.Authorizer == nil { - return genericapiserver.APIGroupInfo{}, false + return genericapiserver.APIGroupInfo{}, false, nil } apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(authorization.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) @@ -54,7 +54,7 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag apiGroupInfo.VersionedResourcesStorageMap[authorizationv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) } - return apiGroupInfo, true + return apiGroupInfo, true, nil } func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { diff --git a/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage.go b/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage.go index eb58e9bc8b0..2a8f90015a6 100644 --- a/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage.go +++ b/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage.go @@ -37,7 +37,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against horizontal pod autoscalers. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &autoscaling.HorizontalPodAutoscaler{} }, NewListFunc: func() runtime.Object { return &autoscaling.HorizontalPodAutoscalerList{} }, @@ -51,12 +51,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = horizontalpodautoscaler.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage_test.go b/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage_test.go index dbb06048c60..a610d38304f 100644 --- a/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage_test.go +++ b/pkg/registry/autoscaling/horizontalpodautoscaler/storage/storage_test.go @@ -45,7 +45,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "horizontalpodautoscalers", } - horizontalPodAutoscalerStorage, statusStorage := NewREST(restOptions) + horizontalPodAutoscalerStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return horizontalPodAutoscalerStorage, statusStorage, server } diff --git a/pkg/registry/autoscaling/rest/storage_autoscaling.go b/pkg/registry/autoscaling/rest/storage_autoscaling.go index b4e527c27ae..05c8ef7ea33 100644 --- a/pkg/registry/autoscaling/rest/storage_autoscaling.go +++ b/pkg/registry/autoscaling/rest/storage_autoscaling.go @@ -31,52 +31,73 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(autoscaling.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(autoscalingapiv2beta2.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv2beta2.SchemeGroupVersion.Version] = p.v2beta2Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v2beta2Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv2beta2.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(autoscalingapiv2beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv2beta1.SchemeGroupVersion.Version] = p.v2beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v2beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv2beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(autoscalingapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // horizontalpodautoscalers - hpaStorage, hpaStatusStorage := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + hpaStorage, hpaStatusStorage, err := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["horizontalpodautoscalers"] = hpaStorage storage["horizontalpodautoscalers/status"] = hpaStatusStorage - return storage + return storage, err } -func (p RESTStorageProvider) v2beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v2beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // horizontalpodautoscalers - hpaStorage, hpaStatusStorage := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + hpaStorage, hpaStatusStorage, err := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["horizontalpodautoscalers"] = hpaStorage storage["horizontalpodautoscalers/status"] = hpaStatusStorage - return storage + return storage, err } -func (p RESTStorageProvider) v2beta2Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v2beta2Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // horizontalpodautoscalers - hpaStorage, hpaStatusStorage := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + hpaStorage, hpaStatusStorage, err := horizontalpodautoscalerstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["horizontalpodautoscalers"] = hpaStorage storage["horizontalpodautoscalers/status"] = hpaStatusStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/batch/cronjob/storage/storage.go b/pkg/registry/batch/cronjob/storage/storage.go index d9d1088c592..7fba3f5d08b 100644 --- a/pkg/registry/batch/cronjob/storage/storage.go +++ b/pkg/registry/batch/cronjob/storage/storage.go @@ -37,7 +37,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against CronJobs. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &batch.CronJob{} }, NewListFunc: func() runtime.Object { return &batch.CronJobList{} }, @@ -51,13 +51,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = cronjob.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } var _ rest.CategoriesProvider = &REST{} diff --git a/pkg/registry/batch/cronjob/storage/storage_test.go b/pkg/registry/batch/cronjob/storage/storage_test.go index 53fbb313856..e2c98d8c14e 100644 --- a/pkg/registry/batch/cronjob/storage/storage_test.go +++ b/pkg/registry/batch/cronjob/storage/storage_test.go @@ -40,7 +40,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "cronjobs", } - storage, statusStorage := NewREST(restOptions) + storage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storage, statusStorage, server } diff --git a/pkg/registry/batch/job/storage/storage.go b/pkg/registry/batch/job/storage/storage.go index 19a157db775..b7390780717 100644 --- a/pkg/registry/batch/job/storage/storage.go +++ b/pkg/registry/batch/job/storage/storage.go @@ -38,13 +38,16 @@ type JobStorage struct { } // NewStorage creates a new JobStorage against etcd. -func NewStorage(optsGetter generic.RESTOptionsGetter) JobStorage { - jobRest, jobStatusRest := NewREST(optsGetter) +func NewStorage(optsGetter generic.RESTOptionsGetter) (JobStorage, error) { + jobRest, jobStatusRest, err := NewREST(optsGetter) + if err != nil { + return JobStorage{}, err + } return JobStorage{ Job: jobRest, Status: jobStatusRest, - } + }, nil } // REST implements a RESTStorage for jobs against etcd @@ -53,7 +56,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against Jobs. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &batch.Job{} }, NewListFunc: func() runtime.Object { return &batch.JobList{} }, @@ -68,13 +71,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: job.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = job.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement CategoriesProvider diff --git a/pkg/registry/batch/job/storage/storage_test.go b/pkg/registry/batch/job/storage/storage_test.go index c15439f8d1b..7974b6a4be1 100644 --- a/pkg/registry/batch/job/storage/storage_test.go +++ b/pkg/registry/batch/job/storage/storage_test.go @@ -39,7 +39,10 @@ func newStorage(t *testing.T) (*JobStorage, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "jobs", } - jobStorage := NewStorage(restOptions) + jobStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return &jobStorage, server } diff --git a/pkg/registry/batch/rest/storage_batch.go b/pkg/registry/batch/rest/storage_batch.go index 4568e8bd929..bffa3f2180e 100644 --- a/pkg/registry/batch/rest/storage_batch.go +++ b/pkg/registry/batch/rest/storage_batch.go @@ -32,52 +32,73 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(batch.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(batchapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[batchapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[batchapiv1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(batchapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[batchapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[batchapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(batchapiv2alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[batchapiv2alpha1.SchemeGroupVersion.Version] = p.v2alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v2alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[batchapiv2alpha1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // jobs - jobsStorage, jobsStatusStorage := jobstore.NewREST(restOptionsGetter) + jobsStorage, jobsStatusStorage, err := jobstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["jobs"] = jobsStorage storage["jobs/status"] = jobsStatusStorage - return storage + return storage, err } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // cronjobs - cronJobsStorage, cronJobsStatusStorage := cronjobstore.NewREST(restOptionsGetter) + cronJobsStorage, cronJobsStatusStorage, err := cronjobstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["cronjobs"] = cronJobsStorage storage["cronjobs/status"] = cronJobsStatusStorage - return storage + return storage, err } -func (p RESTStorageProvider) v2alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v2alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // cronjobs - cronJobsStorage, cronJobsStatusStorage := cronjobstore.NewREST(restOptionsGetter) + cronJobsStorage, cronJobsStatusStorage, err := cronjobstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["cronjobs"] = cronJobsStorage storage["cronjobs/status"] = cronJobsStatusStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/certificates/certificates/storage/storage.go b/pkg/registry/certificates/certificates/storage/storage.go index 953bbd1390c..e7168204439 100644 --- a/pkg/registry/certificates/certificates/storage/storage.go +++ b/pkg/registry/certificates/certificates/storage/storage.go @@ -37,7 +37,7 @@ type REST struct { } // NewREST returns a registry which will store CertificateSigningRequest in the given helper -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *ApprovalREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *ApprovalREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &certificates.CertificateSigningRequest{} }, NewListFunc: func() runtime.Object { return &certificates.CertificateSigningRequestList{} }, @@ -52,7 +52,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Approva } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, nil, err } // Subresources use the same store and creation strategy, which only @@ -64,7 +64,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Approva approvalStore := *store approvalStore.UpdateStrategy = csrregistry.ApprovalStrategy - return &REST{store}, &StatusREST{store: &statusStore}, &ApprovalREST{store: &approvalStore} + return &REST{store}, &StatusREST{store: &statusStore}, &ApprovalREST{store: &approvalStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/certificates/rest/storage_certificates.go b/pkg/registry/certificates/rest/storage_certificates.go index aa1fbc1ae37..12d1a7a4862 100644 --- a/pkg/registry/certificates/rest/storage_certificates.go +++ b/pkg/registry/certificates/rest/storage_certificates.go @@ -29,27 +29,34 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(certificates.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(certificatesapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[certificatesapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[certificatesapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // certificatesigningrequests - csrStorage, csrStatusStorage, csrApprovalStorage := certificatestore.NewREST(restOptionsGetter) + csrStorage, csrStatusStorage, csrApprovalStorage, err := certificatestore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["certificatesigningrequests"] = csrStorage storage["certificatesigningrequests/status"] = csrStatusStorage storage["certificatesigningrequests/approval"] = csrApprovalStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/coordination/lease/storage/storage.go b/pkg/registry/coordination/lease/storage/storage.go index 71589e2b6c3..8989a293a6d 100644 --- a/pkg/registry/coordination/lease/storage/storage.go +++ b/pkg/registry/coordination/lease/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against leases. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &coordinationapi.Lease{} }, NewListFunc: func() runtime.Object { return &coordinationapi.LeaseList{} }, @@ -47,8 +47,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/coordination/rest/storage_coordination.go b/pkg/registry/coordination/rest/storage_coordination.go index 4dae50369de..d89e35c5d29 100644 --- a/pkg/registry/coordination/rest/storage_coordination.go +++ b/pkg/registry/coordination/rest/storage_coordination.go @@ -30,36 +30,50 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(coordination.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(coordinationv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[coordinationv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[coordinationv1beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(coordinationv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[coordinationv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[coordinationv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // leases - leaseStorage := leasestorage.NewREST(restOptionsGetter) + leaseStorage, err := leasestorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["leases"] = leaseStorage - return storage + return storage, err } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // leases - leaseStorage := leasestorage.NewREST(restOptionsGetter) + leaseStorage, err := leasestorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["leases"] = leaseStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/core/configmap/storage/storage.go b/pkg/registry/core/configmap/storage/storage.go index 8456bc10796..b66a073987f 100644 --- a/pkg/registry/core/configmap/storage/storage.go +++ b/pkg/registry/core/configmap/storage/storage.go @@ -35,7 +35,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work with ConfigMap objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ConfigMap{} }, NewListFunc: func() runtime.Object { return &api.ConfigMapList{} }, @@ -53,9 +53,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { TriggerFunc: map[string]storage.IndexerFunc{"metadata.name": configmap.NameTriggerFunc}, } if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/configmap/storage/storage_test.go b/pkg/registry/core/configmap/storage/storage_test.go index b426ab9e5ab..04ccc8d8999 100644 --- a/pkg/registry/core/configmap/storage/storage_test.go +++ b/pkg/registry/core/configmap/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "configmaps", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewConfigMap() *api.ConfigMap { diff --git a/pkg/registry/core/endpoint/storage/storage.go b/pkg/registry/core/endpoint/storage/storage.go index 315b2aa73ee..ee11e09aa6a 100644 --- a/pkg/registry/core/endpoint/storage/storage.go +++ b/pkg/registry/core/endpoint/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against endpoints. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.Endpoints{} }, NewListFunc: func() runtime.Object { return &api.EndpointsList{} }, @@ -47,9 +47,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/endpoint/storage/storage_test.go b/pkg/registry/core/endpoint/storage/storage_test.go index dc6543cfecb..65c3cadf540 100644 --- a/pkg/registry/core/endpoint/storage/storage_test.go +++ b/pkg/registry/core/endpoint/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "endpoints", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewEndpoints() *api.Endpoints { diff --git a/pkg/registry/core/event/storage/storage.go b/pkg/registry/core/event/storage/storage.go index 356ed35d334..0bd41898478 100644 --- a/pkg/registry/core/event/storage/storage.go +++ b/pkg/registry/core/event/storage/storage.go @@ -33,11 +33,11 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against events. -func NewREST(optsGetter generic.RESTOptionsGetter, ttl uint64) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter, ttl uint64) (*REST, error) { resource := api.Resource("events") opts, err := optsGetter.GetRESTOptions(resource) if err != nil { - panic(err) // TODO: Propagate error up + return nil, err } store := &genericregistry.Store{ @@ -57,9 +57,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter, ttl uint64) *REST { } options := &generic.StoreOptions{RESTOptions: opts, AttrFunc: event.GetAttrs} // Pass in opts to use UndecoratedStorage if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/event/storage/storage_test.go b/pkg/registry/core/event/storage/storage_test.go index 08d3e9c2378..c17323d713b 100644 --- a/pkg/registry/core/event/storage/storage_test.go +++ b/pkg/registry/core/event/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "events", } - return NewREST(restOptions, testTTL), server + rest, err := NewREST(restOptions, testTTL) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewEvent(namespace string) *api.Event { diff --git a/pkg/registry/core/limitrange/storage/storage.go b/pkg/registry/core/limitrange/storage/storage.go index adb60f252db..0315e4fa7ce 100644 --- a/pkg/registry/core/limitrange/storage/storage.go +++ b/pkg/registry/core/limitrange/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against limitranges. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.LimitRange{} }, NewListFunc: func() runtime.Object { return &api.LimitRangeList{} }, @@ -43,9 +43,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/limitrange/storage/storage_test.go b/pkg/registry/core/limitrange/storage/storage_test.go index 6cff9d2736c..7a4fedede2c 100644 --- a/pkg/registry/core/limitrange/storage/storage_test.go +++ b/pkg/registry/core/limitrange/storage/storage_test.go @@ -39,7 +39,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "limitranges", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewLimitRange() *api.LimitRange { diff --git a/pkg/registry/core/namespace/storage/storage.go b/pkg/registry/core/namespace/storage/storage.go index 7a7b3a646fe..ef1c1f7bdf0 100644 --- a/pkg/registry/core/namespace/storage/storage.go +++ b/pkg/registry/core/namespace/storage/storage.go @@ -58,7 +58,7 @@ type FinalizeREST struct { } // NewREST returns a RESTStorage object that will work against namespaces. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *FinalizeREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *FinalizeREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.Namespace{} }, NewListFunc: func() runtime.Object { return &api.NamespaceList{} }, @@ -76,7 +76,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Finaliz } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: namespace.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, nil, err } statusStore := *store @@ -85,7 +85,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Finaliz finalizeStore := *store finalizeStore.UpdateStrategy = namespace.FinalizeStrategy - return &REST{store: store, status: &statusStore}, &StatusREST{store: &statusStore}, &FinalizeREST{store: &finalizeStore} + return &REST{store: store, status: &statusStore}, &StatusREST{store: &statusStore}, &FinalizeREST{store: &finalizeStore}, nil } func (r *REST) NamespaceScoped() bool { diff --git a/pkg/registry/core/namespace/storage/storage_test.go b/pkg/registry/core/namespace/storage/storage_test.go index 6912f1c3b69..61e3dedee78 100644 --- a/pkg/registry/core/namespace/storage/storage_test.go +++ b/pkg/registry/core/namespace/storage/storage_test.go @@ -37,7 +37,10 @@ import ( func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, "") restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "namespaces"} - namespaceStorage, _, _ := NewREST(restOptions) + namespaceStorage, _, _, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return namespaceStorage, server } @@ -278,7 +281,10 @@ func TestFinalizeDeletingNamespaceWithCompleteFinalizers(t *testing.T) { // get finalize storage etcdStorage, server := registrytest.NewEtcdStorage(t, "") restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "namespaces"} - storage, _, finalizeStorage := NewREST(restOptions) + storage, _, finalizeStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } defer server.Terminate(t) defer storage.store.DestroyFunc() @@ -318,7 +324,10 @@ func TestFinalizeDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T) // get finalize storage etcdStorage, server := registrytest.NewEtcdStorage(t, "") restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "namespaces"} - storage, _, finalizeStorage := NewREST(restOptions) + storage, _, finalizeStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } defer server.Terminate(t) defer storage.store.DestroyFunc() diff --git a/pkg/registry/core/persistentvolume/storage/storage.go b/pkg/registry/core/persistentvolume/storage/storage.go index 14b4640c66a..eeeb471c930 100644 --- a/pkg/registry/core/persistentvolume/storage/storage.go +++ b/pkg/registry/core/persistentvolume/storage/storage.go @@ -36,7 +36,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against persistent volumes. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.PersistentVolume{} }, NewListFunc: func() runtime.Object { return &api.PersistentVolumeList{} }, @@ -52,13 +52,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: persistentvolume.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = persistentvolume.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/persistentvolume/storage/storage_test.go b/pkg/registry/core/persistentvolume/storage/storage_test.go index 5fa67356717..62f15d35e70 100644 --- a/pkg/registry/core/persistentvolume/storage/storage_test.go +++ b/pkg/registry/core/persistentvolume/storage/storage_test.go @@ -43,7 +43,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "persistentvolumes", } - persistentVolumeStorage, statusStorage := NewREST(restOptions) + persistentVolumeStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return persistentVolumeStorage, statusStorage, server } diff --git a/pkg/registry/core/persistentvolumeclaim/storage/storage.go b/pkg/registry/core/persistentvolumeclaim/storage/storage.go index 7e0481941cf..bf18473e400 100644 --- a/pkg/registry/core/persistentvolumeclaim/storage/storage.go +++ b/pkg/registry/core/persistentvolumeclaim/storage/storage.go @@ -36,7 +36,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against persistent volume claims. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.PersistentVolumeClaim{} }, NewListFunc: func() runtime.Object { return &api.PersistentVolumeClaimList{} }, @@ -52,13 +52,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: persistentvolumeclaim.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = persistentvolumeclaim.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/persistentvolumeclaim/storage/storage_test.go b/pkg/registry/core/persistentvolumeclaim/storage/storage_test.go index ccee385510b..c023a4e2360 100644 --- a/pkg/registry/core/persistentvolumeclaim/storage/storage_test.go +++ b/pkg/registry/core/persistentvolumeclaim/storage/storage_test.go @@ -43,7 +43,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "persistentvolumeclaims", } - persistentVolumeClaimStorage, statusStorage := NewREST(restOptions) + persistentVolumeClaimStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return persistentVolumeClaimStorage, statusStorage, server } diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go index 11a1dfed685..c6890291e45 100644 --- a/pkg/registry/core/pod/storage/eviction_test.go +++ b/pkg/registry/core/pod/storage/eviction_test.go @@ -170,7 +170,10 @@ func newFailDeleteUpdateStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServ DeleteCollectionWorkers: 3, ResourcePrefix: "pods", } - storage := NewStorage(restOptions, nil, nil, nil) + storage, err := NewStorage(restOptions, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } storage.Pod.Store.Storage = genericregistry.DryRunnableStorage{ Storage: FailDeleteUpdateStorage{storage.Pod.Store.Storage.Storage}, Codec: apitesting.TestStorageCodec(codecs, examplev1.SchemeGroupVersion), diff --git a/pkg/registry/core/pod/storage/storage.go b/pkg/registry/core/pod/storage/storage.go index b9cc768b2b2..55abd49bd38 100644 --- a/pkg/registry/core/pod/storage/storage.go +++ b/pkg/registry/core/pod/storage/storage.go @@ -66,7 +66,7 @@ type REST struct { } // NewStorage returns a RESTStorage object that will work against pods. -func NewStorage(optsGetter generic.RESTOptionsGetter, k client.ConnectionInfoGetter, proxyTransport http.RoundTripper, podDisruptionBudgetClient policyclient.PodDisruptionBudgetsGetter) PodStorage { +func NewStorage(optsGetter generic.RESTOptionsGetter, k client.ConnectionInfoGetter, proxyTransport http.RoundTripper, podDisruptionBudgetClient policyclient.PodDisruptionBudgetsGetter) (PodStorage, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.Pod{} }, @@ -87,7 +87,7 @@ func NewStorage(optsGetter generic.RESTOptionsGetter, k client.ConnectionInfoGet TriggerFunc: map[string]storage.IndexerFunc{"spec.nodeName": pod.NodeNameTriggerFunc}, } if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return PodStorage{}, err } statusStore := *store @@ -106,7 +106,7 @@ func NewStorage(optsGetter generic.RESTOptionsGetter, k client.ConnectionInfoGet Exec: &podrest.ExecREST{Store: store, KubeletConn: k}, Attach: &podrest.AttachREST{Store: store, KubeletConn: k}, PortForward: &podrest.PortForwardREST{Store: store, KubeletConn: k}, - } + }, nil } // Implement Redirector. diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 515c1edee89..d8c8e324f7c 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -52,7 +52,10 @@ func newStorage(t *testing.T) (*REST, *BindingREST, *StatusREST, *etcd3testing.E DeleteCollectionWorkers: 3, ResourcePrefix: "pods", } - storage := NewStorage(restOptions, nil, nil, nil) + storage, err := NewStorage(restOptions, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storage.Pod, storage.Binding, storage.Status, server } @@ -169,7 +172,10 @@ func newFailDeleteStorage(t *testing.T, called *bool) (*REST, *etcd3testing.Etcd DeleteCollectionWorkers: 3, ResourcePrefix: "pods", } - storage := NewStorage(restOptions, nil, nil, nil) + storage, err := NewStorage(restOptions, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } storage.Pod.Store.Storage = genericregistry.DryRunnableStorage{Storage: FailDeletionStorage{storage.Pod.Store.Storage.Storage, called}} return storage.Pod, server } diff --git a/pkg/registry/core/podtemplate/storage/storage.go b/pkg/registry/core/podtemplate/storage/storage.go index 73447cdc804..495e0994a5c 100644 --- a/pkg/registry/core/podtemplate/storage/storage.go +++ b/pkg/registry/core/podtemplate/storage/storage.go @@ -32,7 +32,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against pod templates. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.PodTemplate{} }, NewListFunc: func() runtime.Object { return &api.PodTemplateList{} }, @@ -49,7 +49,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/core/podtemplate/storage/storage_test.go b/pkg/registry/core/podtemplate/storage/storage_test.go index a85f7bc2f9d..785fc6fcb62 100644 --- a/pkg/registry/core/podtemplate/storage/storage_test.go +++ b/pkg/registry/core/podtemplate/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "podtemplates", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewPodTemplate(name string) *api.PodTemplate { diff --git a/pkg/registry/core/replicationcontroller/storage/storage.go b/pkg/registry/core/replicationcontroller/storage/storage.go index f8b04fe1631..05887726848 100644 --- a/pkg/registry/core/replicationcontroller/storage/storage.go +++ b/pkg/registry/core/replicationcontroller/storage/storage.go @@ -48,14 +48,17 @@ type ControllerStorage struct { Scale *ScaleREST } -func NewStorage(optsGetter generic.RESTOptionsGetter) ControllerStorage { - controllerREST, statusREST := NewREST(optsGetter) +func NewStorage(optsGetter generic.RESTOptionsGetter) (ControllerStorage, error) { + controllerREST, statusREST, err := NewREST(optsGetter) + if err != nil { + return ControllerStorage{}, err + } return ControllerStorage{ Controller: controllerREST, Status: statusREST, Scale: &ScaleREST{store: controllerREST.Store}, - } + }, nil } type REST struct { @@ -63,7 +66,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against replication controllers. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ReplicationController{} }, NewListFunc: func() runtime.Object { return &api.ReplicationControllerList{} }, @@ -78,13 +81,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: replicationcontroller.GetAttrs} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = replicationcontroller.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/replicationcontroller/storage/storage_test.go b/pkg/registry/core/replicationcontroller/storage/storage_test.go index b9bcae002b2..3bd23598452 100644 --- a/pkg/registry/core/replicationcontroller/storage/storage_test.go +++ b/pkg/registry/core/replicationcontroller/storage/storage_test.go @@ -49,7 +49,10 @@ func newStorage(t *testing.T) (ControllerStorage, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "replicationcontrollers", } - storage := NewStorage(restOptions) + storage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storage, server } diff --git a/pkg/registry/core/resourcequota/storage/storage.go b/pkg/registry/core/resourcequota/storage/storage.go index 7e4cf2b1a89..6af4444a4a3 100644 --- a/pkg/registry/core/resourcequota/storage/storage.go +++ b/pkg/registry/core/resourcequota/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against resource quotas. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ResourceQuota{} }, NewListFunc: func() runtime.Object { return &api.ResourceQuotaList{} }, @@ -46,13 +46,13 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = resourcequota.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/resourcequota/storage/storage_test.go b/pkg/registry/core/resourcequota/storage/storage_test.go index e3a4a8155f5..252f8e9bb24 100644 --- a/pkg/registry/core/resourcequota/storage/storage_test.go +++ b/pkg/registry/core/resourcequota/storage/storage_test.go @@ -42,7 +42,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "resourcequotas", } - resourceQuotaStorage, statusStorage := NewREST(restOptions) + resourceQuotaStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return resourceQuotaStorage, statusStorage, server } diff --git a/pkg/registry/core/rest/storage_core.go b/pkg/registry/core/rest/storage_core.go index b6308e90337..7dd9cdcd341 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -115,41 +115,80 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi } restStorage := LegacyRESTStorage{} - podTemplateStorage := podtemplatestore.NewREST(restOptionsGetter) + podTemplateStorage, err := podtemplatestore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } - eventStorage := eventstore.NewREST(restOptionsGetter, uint64(c.EventTTL.Seconds())) - limitRangeStorage := limitrangestore.NewREST(restOptionsGetter) + eventStorage, err := eventstore.NewREST(restOptionsGetter, uint64(c.EventTTL.Seconds())) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } + limitRangeStorage, err := limitrangestore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } - resourceQuotaStorage, resourceQuotaStatusStorage := resourcequotastore.NewREST(restOptionsGetter) - secretStorage := secretstore.NewREST(restOptionsGetter) - persistentVolumeStorage, persistentVolumeStatusStorage := pvstore.NewREST(restOptionsGetter) - persistentVolumeClaimStorage, persistentVolumeClaimStatusStorage := pvcstore.NewREST(restOptionsGetter) - configMapStorage := configmapstore.NewREST(restOptionsGetter) + resourceQuotaStorage, resourceQuotaStatusStorage, err := resourcequotastore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } + secretStorage, err := secretstore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } + persistentVolumeStorage, persistentVolumeStatusStorage, err := pvstore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } + persistentVolumeClaimStorage, persistentVolumeClaimStatusStorage, err := pvcstore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } + configMapStorage, err := configmapstore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } - namespaceStorage, namespaceStatusStorage, namespaceFinalizeStorage := namespacestore.NewREST(restOptionsGetter) + namespaceStorage, namespaceStatusStorage, namespaceFinalizeStorage, err := namespacestore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } - endpointsStorage := endpointsstore.NewREST(restOptionsGetter) + endpointsStorage, err := endpointsstore.NewREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } nodeStorage, err := nodestore.NewStorage(restOptionsGetter, c.KubeletClientConfig, c.ProxyTransport) if err != nil { return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err } - podStorage := podstore.NewStorage( + podStorage, err := podstore.NewStorage( restOptionsGetter, nodeStorage.KubeletConnectionInfo, c.ProxyTransport, podDisruptionClient, ) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } var serviceAccountStorage *serviceaccountstore.REST if c.ServiceAccountIssuer != nil && utilfeature.DefaultFeatureGate.Enabled(features.TokenRequest) { - serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.APIAudiences, c.ServiceAccountMaxExpiration, podStorage.Pod.Store, secretStorage.Store) + serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.APIAudiences, c.ServiceAccountMaxExpiration, podStorage.Pod.Store, secretStorage.Store) } else { - serviceAccountStorage = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 0, nil, nil) + serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 0, nil, nil) + } + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err } - serviceRESTStorage, serviceStatusStorage := servicestore.NewGenericREST(restOptionsGetter) + serviceRESTStorage, serviceStatusStorage, err := servicestore.NewGenericREST(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } var serviceClusterIPRegistry rangeallocation.RangeRegistry serviceClusterIPRange := c.ServiceIPRange @@ -181,7 +220,10 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi }) restStorage.ServiceNodePortAllocator = serviceNodePortRegistry - controllerStorage := controllerstore.NewStorage(restOptionsGetter) + controllerStorage, err := controllerstore.NewStorage(restOptionsGetter) + if err != nil { + return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err + } serviceRest, serviceRestProxy := servicestore.NewREST(serviceRESTStorage, endpointsStorage, podStorage.Pod, serviceClusterIPAllocator, serviceNodePortAllocator, c.ProxyTransport) diff --git a/pkg/registry/core/secret/storage/storage.go b/pkg/registry/core/secret/storage/storage.go index aa2dd5ac73f..4d3c7114b47 100644 --- a/pkg/registry/core/secret/storage/storage.go +++ b/pkg/registry/core/secret/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against secrets. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.Secret{} }, NewListFunc: func() runtime.Object { return &api.SecretList{} }, @@ -53,7 +53,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { TriggerFunc: map[string]storage.IndexerFunc{"metadata.name": secret.NameTriggerFunc}, } if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/core/secret/storage/storage_test.go b/pkg/registry/core/secret/storage/storage_test.go index 27a665086ea..a3850a2dafe 100644 --- a/pkg/registry/core/secret/storage/storage_test.go +++ b/pkg/registry/core/secret/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "secrets", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewSecret(name string) *api.Secret { diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index d8e6bdb1067..dace2102874 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -176,12 +176,15 @@ func NewTestRESTWithPods(t *testing.T, endpoints *api.EndpointsList, pods *api.P serviceStorage := &serviceStorage{} - podStorage := podstore.NewStorage(generic.RESTOptions{ + podStorage, err := podstore.NewStorage(generic.RESTOptions{ StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 3, ResourcePrefix: "pods", }, nil, nil, nil) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } if pods != nil && len(pods.Items) > 0 { ctx := genericapirequest.NewDefaultContext() for ix := range pods.Items { @@ -191,11 +194,14 @@ func NewTestRESTWithPods(t *testing.T, endpoints *api.EndpointsList, pods *api.P } } } - endpointStorage := endpointstore.NewREST(generic.RESTOptions{ + endpointStorage, err := endpointstore.NewREST(generic.RESTOptions{ StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, ResourcePrefix: "endpoints", }) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } if endpoints != nil && len(endpoints.Items) > 0 { ctx := genericapirequest.NewDefaultContext() for ix := range endpoints.Items { diff --git a/pkg/registry/core/service/storage/storage.go b/pkg/registry/core/service/storage/storage.go index df89d04fd92..fa557935a05 100644 --- a/pkg/registry/core/service/storage/storage.go +++ b/pkg/registry/core/service/storage/storage.go @@ -36,7 +36,7 @@ type GenericREST struct { } // NewREST returns a RESTStorage object that will work against services. -func NewGenericREST(optsGetter generic.RESTOptionsGetter) (*GenericREST, *StatusREST) { +func NewGenericREST(optsGetter generic.RESTOptionsGetter) (*GenericREST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.Service{} }, NewListFunc: func() runtime.Object { return &api.ServiceList{} }, @@ -52,12 +52,12 @@ func NewGenericREST(optsGetter generic.RESTOptionsGetter) (*GenericREST, *Status } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = service.StatusStrategy - return &GenericREST{store}, &StatusREST{store: &statusStore} + return &GenericREST{store}, &StatusREST{store: &statusStore}, nil } var ( diff --git a/pkg/registry/core/service/storage/storage_test.go b/pkg/registry/core/service/storage/storage_test.go index b51cd2637a2..0b8d2f1b6cb 100644 --- a/pkg/registry/core/service/storage/storage_test.go +++ b/pkg/registry/core/service/storage/storage_test.go @@ -39,7 +39,10 @@ func newStorage(t *testing.T) (*GenericREST, *StatusREST, *etcd3testing.EtcdTest DeleteCollectionWorkers: 1, ResourcePrefix: "services", } - serviceStorage, statusStorage := NewGenericREST(restOptions) + serviceStorage, statusStorage, err := NewGenericREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return serviceStorage, statusStorage, server } diff --git a/pkg/registry/core/serviceaccount/storage/storage.go b/pkg/registry/core/serviceaccount/storage/storage.go index c9d2832e97f..3667fcb7310 100644 --- a/pkg/registry/core/serviceaccount/storage/storage.go +++ b/pkg/registry/core/serviceaccount/storage/storage.go @@ -38,7 +38,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against service accounts. -func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds authenticator.Audiences, max time.Duration, podStorage, secretStorage *genericregistry.Store) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds authenticator.Audiences, max time.Duration, podStorage, secretStorage *genericregistry.Store) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ServiceAccount{} }, NewListFunc: func() runtime.Object { return &api.ServiceAccountList{} }, @@ -53,7 +53,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } var trest *TokenREST @@ -71,7 +71,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, return &REST{ Store: store, Token: trest, - } + }, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/core/serviceaccount/storage/storage_test.go b/pkg/registry/core/serviceaccount/storage/storage_test.go index 689c8e45207..605f0e5ba40 100644 --- a/pkg/registry/core/serviceaccount/storage/storage_test.go +++ b/pkg/registry/core/serviceaccount/storage/storage_test.go @@ -38,7 +38,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "serviceaccounts", } - return NewREST(restOptions, nil, nil, 0, nil, nil), server + rest, err := NewREST(restOptions, nil, nil, 0, nil, nil) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewServiceAccount(name string) *api.ServiceAccount { diff --git a/pkg/registry/events/rest/storage_events.go b/pkg/registry/events/rest/storage_events.go index de4caba3839..0c9480e63d1 100644 --- a/pkg/registry/events/rest/storage_events.go +++ b/pkg/registry/events/rest/storage_events.go @@ -33,25 +33,32 @@ type RESTStorageProvider struct { TTL time.Duration } -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(events.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(eventsapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[eventsapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[eventsapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // events - eventsStorage := eventstore.NewREST(restOptionsGetter, uint64(p.TTL.Seconds())) + eventsStorage, err := eventstore.NewREST(restOptionsGetter, uint64(p.TTL.Seconds())) + if err != nil { + return storage, err + } storage["events"] = eventsStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/extensions/controller/storage/storage.go b/pkg/registry/extensions/controller/storage/storage.go index 6c7190d65c1..2c710064fee 100644 --- a/pkg/registry/extensions/controller/storage/storage.go +++ b/pkg/registry/extensions/controller/storage/storage.go @@ -40,14 +40,17 @@ type ContainerStorage struct { Scale *ScaleREST } -func NewStorage(optsGetter generic.RESTOptionsGetter) ContainerStorage { +func NewStorage(optsGetter generic.RESTOptionsGetter) (ContainerStorage, error) { // scale does not set status, only updates spec so we ignore the status - controllerREST, _ := controllerstore.NewREST(optsGetter) + controllerREST, _, err := controllerstore.NewREST(optsGetter) + if err != nil { + return ContainerStorage{}, err + } return ContainerStorage{ ReplicationController: &RcREST{}, Scale: &ScaleREST{store: controllerREST.Store}, - } + }, nil } type ScaleREST struct { diff --git a/pkg/registry/extensions/controller/storage/storage_test.go b/pkg/registry/extensions/controller/storage/storage_test.go index bf22fe5481f..7cc575d4f42 100644 --- a/pkg/registry/extensions/controller/storage/storage_test.go +++ b/pkg/registry/extensions/controller/storage/storage_test.go @@ -42,7 +42,11 @@ func newStorage(t *testing.T) (*ScaleREST, *etcd3testing.EtcdTestServer, storage d() server.Terminate(t) } - return NewStorage(restOptions).Scale, server, s, destroyFunc + storage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return storage.Scale, server, s, destroyFunc } var validPodTemplate = api.PodTemplate{ diff --git a/pkg/registry/extensions/rest/storage_extensions.go b/pkg/registry/extensions/rest/storage_extensions.go index 0a1fa286dcb..458e8f899a7 100644 --- a/pkg/registry/extensions/rest/storage_extensions.go +++ b/pkg/registry/extensions/rest/storage_extensions.go @@ -35,39 +35,52 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(extensions.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(extensionsapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[extensionsapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[extensionsapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // This is a dummy replication controller for scale subresource purposes. // TODO: figure out how to enable this only if needed as a part of scale subresource GA. if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicationcontrollers")) { - controllerStorage := expcontrollerstore.NewStorage(restOptionsGetter) + controllerStorage, err := expcontrollerstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["replicationcontrollers"] = controllerStorage.ReplicationController storage["replicationcontrollers/scale"] = controllerStorage.Scale } // daemonsets if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets")) { - daemonSetStorage, daemonSetStatusStorage := daemonstore.NewREST(restOptionsGetter) + daemonSetStorage, daemonSetStatusStorage, err := daemonstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["daemonsets"] = daemonSetStorage.WithCategories(nil) storage["daemonsets/status"] = daemonSetStatusStorage } //deployments if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments")) { - deploymentStorage := deploymentstore.NewStorage(restOptionsGetter) + deploymentStorage, err := deploymentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["deployments"] = deploymentStorage.Deployment.WithCategories(nil) storage["deployments/status"] = deploymentStorage.Status storage["deployments/rollback"] = deploymentStorage.Rollback @@ -75,20 +88,29 @@ func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorag } // ingresses if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses")) { - ingressStorage, ingressStatusStorage := ingressstore.NewREST(restOptionsGetter) + ingressStorage, ingressStatusStorage, err := ingressstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["ingresses"] = ingressStorage storage["ingresses/status"] = ingressStatusStorage } // podsecuritypolicy if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("podsecuritypolicies")) { - podSecurityPolicyStorage := pspstore.NewREST(restOptionsGetter) + podSecurityPolicyStorage, err := pspstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["podSecurityPolicies"] = podSecurityPolicyStorage } // replicasets if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets")) { - replicaSetStorage := replicasetstore.NewStorage(restOptionsGetter) + replicaSetStorage, err := replicasetstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["replicasets"] = replicaSetStorage.ReplicaSet.WithCategories(nil) storage["replicasets/status"] = replicaSetStorage.Status storage["replicasets/scale"] = replicaSetStorage.Scale @@ -96,11 +118,14 @@ func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorag // networkpolicies if apiResourceConfigSource.ResourceEnabled(extensionsapiv1beta1.SchemeGroupVersion.WithResource("networkpolicies")) { - networkExtensionsStorage := networkpolicystore.NewREST(restOptionsGetter) + networkExtensionsStorage, err := networkpolicystore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["networkpolicies"] = networkExtensionsStorage } - return storage + return storage, nil } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/networking/ingress/storage/storage.go b/pkg/registry/networking/ingress/storage/storage.go index 5adefd9b22b..6c04f71ed32 100644 --- a/pkg/registry/networking/ingress/storage/storage.go +++ b/pkg/registry/networking/ingress/storage/storage.go @@ -37,7 +37,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against replication controllers. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &networking.Ingress{} }, NewListFunc: func() runtime.Object { return &networking.IngressList{} }, @@ -51,12 +51,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = ingress.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/networking/ingress/storage/storage_test.go b/pkg/registry/networking/ingress/storage/storage_test.go index 715e4fc4e4a..79483b09767 100644 --- a/pkg/registry/networking/ingress/storage/storage_test.go +++ b/pkg/registry/networking/ingress/storage/storage_test.go @@ -40,7 +40,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "ingresses", } - ingressStorage, statusStorage := NewREST(restOptions) + ingressStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return ingressStorage, statusStorage, server } diff --git a/pkg/registry/networking/networkpolicy/storage/storage.go b/pkg/registry/networking/networkpolicy/storage/storage.go index ba3e87aa4b0..93e200c7a31 100644 --- a/pkg/registry/networking/networkpolicy/storage/storage.go +++ b/pkg/registry/networking/networkpolicy/storage/storage.go @@ -34,7 +34,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against NetworkPolicies -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &networkingapi.NetworkPolicy{} }, NewListFunc: func() runtime.Object { return &networkingapi.NetworkPolicyList{} }, @@ -48,10 +48,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/networking/networkpolicy/storage/storage_test.go b/pkg/registry/networking/networkpolicy/storage/storage_test.go index 80e3c25fea2..e1dea17e585 100644 --- a/pkg/registry/networking/networkpolicy/storage/storage_test.go +++ b/pkg/registry/networking/networkpolicy/storage/storage_test.go @@ -40,7 +40,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "networkpolicies", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNetworkPolicy() *networking.NetworkPolicy { diff --git a/pkg/registry/networking/rest/storage_settings.go b/pkg/registry/networking/rest/storage_settings.go index fc892bf66ec..21d0de5ee3c 100644 --- a/pkg/registry/networking/rest/storage_settings.go +++ b/pkg/registry/networking/rest/storage_settings.go @@ -31,38 +31,52 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(networking.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(networkingapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[networkingapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[networkingapiv1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(networkingapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[networkingapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[networkingapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // networkpolicies - networkPolicyStorage := networkpolicystore.NewREST(restOptionsGetter) + networkPolicyStorage, err := networkpolicystore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["networkpolicies"] = networkPolicyStorage - return storage + return storage, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // ingresses - ingressStorage, ingressStatusStorage := ingressstore.NewREST(restOptionsGetter) + ingressStorage, ingressStatusStorage, err := ingressstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["ingresses"] = ingressStorage storage["ingresses/status"] = ingressStatusStorage - return storage + return storage, nil } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/node/rest/runtime_class.go b/pkg/registry/node/rest/runtime_class.go index 5d242262530..5a8f857f39a 100644 --- a/pkg/registry/node/rest/runtime_class.go +++ b/pkg/registry/node/rest/runtime_class.go @@ -32,34 +32,48 @@ import ( type RESTStorageProvider struct{} // NewRESTStorage returns a RESTStorageProvider -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(nodeinternal.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) if apiResourceConfigSource.VersionEnabled(nodev1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[nodev1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[nodev1alpha1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(nodev1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[nodev1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[nodev1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} - s := runtimeclassstorage.NewREST(restOptionsGetter) + s, err := runtimeclassstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["runtimeclasses"] = s - return storage + return storage, err } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} - s := runtimeclassstorage.NewREST(restOptionsGetter) + s, err := runtimeclassstorage.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["runtimeclasses"] = s - return storage + return storage, err } // GroupName is the group name for the storage provider diff --git a/pkg/registry/node/runtimeclass/storage/storage.go b/pkg/registry/node/runtimeclass/storage/storage.go index 75951c5367b..1388205a1c5 100644 --- a/pkg/registry/node/runtimeclass/storage/storage.go +++ b/pkg/registry/node/runtimeclass/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against runtime classes. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &node.RuntimeClass{} }, NewListFunc: func() runtime.Object { return &node.RuntimeClassList{} }, @@ -45,7 +45,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/policy/poddisruptionbudget/storage/storage.go b/pkg/registry/policy/poddisruptionbudget/storage/storage.go index cf4d9873b49..8e13b902d43 100644 --- a/pkg/registry/policy/poddisruptionbudget/storage/storage.go +++ b/pkg/registry/policy/poddisruptionbudget/storage/storage.go @@ -37,7 +37,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against pod disruption budgets. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &policyapi.PodDisruptionBudget{} }, NewListFunc: func() runtime.Object { return &policyapi.PodDisruptionBudgetList{} }, @@ -51,12 +51,12 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, nil, err } statusStore := *store statusStore.UpdateStrategy = poddisruptionbudget.StatusStrategy - return &REST{store}, &StatusREST{store: &statusStore} + return &REST{store}, &StatusREST{store: &statusStore}, nil } // ShortNames implements the ShortNamesProvider interface. Returns a list of short names for a resource. diff --git a/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go b/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go index ea99ea4b79f..e9ca406710d 100644 --- a/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go +++ b/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go @@ -35,7 +35,10 @@ import ( func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorage(t, policy.GroupName) restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "poddisruptionbudgets"} - podDisruptionBudgetStorage, statusStorage := NewREST(restOptions) + podDisruptionBudgetStorage, statusStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return podDisruptionBudgetStorage, statusStorage, server } diff --git a/pkg/registry/policy/podsecuritypolicy/storage/storage.go b/pkg/registry/policy/podsecuritypolicy/storage/storage.go index dfb132e5027..1b916ad6ce9 100644 --- a/pkg/registry/policy/podsecuritypolicy/storage/storage.go +++ b/pkg/registry/policy/podsecuritypolicy/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against PodSecurityPolicy objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &policy.PodSecurityPolicy{} }, NewListFunc: func() runtime.Object { return &policy.PodSecurityPolicyList{} }, @@ -48,9 +48,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // ShortNames implements the ShortNamesProvider interface. Returns a list of short names for a resource. diff --git a/pkg/registry/policy/podsecuritypolicy/storage/storage_test.go b/pkg/registry/policy/podsecuritypolicy/storage/storage_test.go index 8f19df90650..ebcfbb45eb0 100644 --- a/pkg/registry/policy/podsecuritypolicy/storage/storage_test.go +++ b/pkg/registry/policy/podsecuritypolicy/storage/storage_test.go @@ -40,7 +40,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "podsecuritypolicies", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewPodSecurityPolicy() *policy.PodSecurityPolicy { diff --git a/pkg/registry/policy/rest/storage_policy.go b/pkg/registry/policy/rest/storage_policy.go index a4ddb9591d9..f71b523e4ee 100644 --- a/pkg/registry/policy/rest/storage_policy.go +++ b/pkg/registry/policy/rest/storage_policy.go @@ -30,27 +30,38 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(policy.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(policyapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[policyapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[policyapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // poddisruptionbudgets - poddisruptionbudgetStorage, poddisruptionbudgetStatusStorage := poddisruptionbudgetstore.NewREST(restOptionsGetter) + poddisruptionbudgetStorage, poddisruptionbudgetStatusStorage, err := poddisruptionbudgetstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["poddisruptionbudgets"] = poddisruptionbudgetStorage storage["poddisruptionbudgets/status"] = poddisruptionbudgetStatusStorage - storage["podsecuritypolicies"] = pspstore.NewREST(restOptionsGetter) + rest, err := pspstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } + storage["podsecuritypolicies"] = rest - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/rbac/clusterrole/storage/storage.go b/pkg/registry/rbac/clusterrole/storage/storage.go index 940713f305a..7a32c8d7b5e 100644 --- a/pkg/registry/rbac/clusterrole/storage/storage.go +++ b/pkg/registry/rbac/clusterrole/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against ClusterRole objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &rbac.ClusterRole{} }, NewListFunc: func() runtime.Object { return &rbac.ClusterRoleList{} }, @@ -42,8 +42,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/rbac/clusterrolebinding/storage/storage.go b/pkg/registry/rbac/clusterrolebinding/storage/storage.go index 709b50681a5..762730788b6 100644 --- a/pkg/registry/rbac/clusterrolebinding/storage/storage.go +++ b/pkg/registry/rbac/clusterrolebinding/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against ClusterRoleBinding objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &rbac.ClusterRoleBinding{} }, NewListFunc: func() runtime.Object { return &rbac.ClusterRoleBindingList{} }, @@ -47,8 +47,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/rbac/rest/storage_rbac.go b/pkg/registry/rbac/rest/storage_rbac.go index 812799a287f..a887cdeb8dc 100644 --- a/pkg/registry/rbac/rest/storage_rbac.go +++ b/pkg/registry/rbac/rest/storage_rbac.go @@ -65,30 +65,54 @@ type RESTStorageProvider struct { var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(rbac.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(rbacapiv1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1alpha1.SchemeGroupVersion.Version] = p.storage(rbacapiv1alpha1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.storage(rbacapiv1alpha1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1alpha1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(rbacapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1beta1.SchemeGroupVersion.Version] = p.storage(rbacapiv1beta1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter) + if stoageMap, err := p.storage(rbacapiv1beta1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1beta1.SchemeGroupVersion.Version] = stoageMap + } } if apiResourceConfigSource.VersionEnabled(rbacapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1.SchemeGroupVersion.Version] = p.storage(rbacapiv1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.storage(rbacapiv1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) storage(version schema.GroupVersion, apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) storage(version schema.GroupVersion, apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} - rolesStorage := rolestore.NewREST(restOptionsGetter) - roleBindingsStorage := rolebindingstore.NewREST(restOptionsGetter) - clusterRolesStorage := clusterrolestore.NewREST(restOptionsGetter) - clusterRoleBindingsStorage := clusterrolebindingstore.NewREST(restOptionsGetter) + rolesStorage, err := rolestore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } + roleBindingsStorage, err := rolebindingstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } + clusterRolesStorage, err := clusterrolestore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } + clusterRoleBindingsStorage, err := clusterrolebindingstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } authorizationRuleResolver := rbacregistryvalidation.NewDefaultRuleResolver( role.AuthorizerAdapter{Registry: role.NewRegistry(rolesStorage)}, @@ -109,7 +133,7 @@ func (p RESTStorageProvider) storage(version schema.GroupVersion, apiResourceCon // clusterrolebindings storage["clusterrolebindings"] = clusterrolebindingpolicybased.NewStorage(clusterRoleBindingsStorage, p.Authorizer, authorizationRuleResolver) - return storage + return storage, nil } func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStartHookFunc, error) { diff --git a/pkg/registry/rbac/role/storage/storage.go b/pkg/registry/rbac/role/storage/storage.go index 6f151879d1f..1c94fb89f95 100644 --- a/pkg/registry/rbac/role/storage/storage.go +++ b/pkg/registry/rbac/role/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against Role objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &rbac.Role{} }, NewListFunc: func() runtime.Object { return &rbac.RoleList{} }, @@ -42,8 +42,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/rbac/rolebinding/storage/storage.go b/pkg/registry/rbac/rolebinding/storage/storage.go index cce71ccb7ad..0cbcfa0f644 100644 --- a/pkg/registry/rbac/rolebinding/storage/storage.go +++ b/pkg/registry/rbac/rolebinding/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against RoleBinding objects. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &rbac.RoleBinding{} }, NewListFunc: func() runtime.Object { return &rbac.RoleBindingList{} }, @@ -47,8 +47,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/scheduling/priorityclass/storage/storage.go b/pkg/registry/scheduling/priorityclass/storage/storage.go index 69e547116ce..7f13b7a5fa7 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage.go @@ -19,7 +19,6 @@ package storage import ( "context" "errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -39,7 +38,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against priority classes. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &scheduling.PriorityClass{} }, NewListFunc: func() runtime.Object { return &scheduling.PriorityClassList{} }, @@ -53,10 +52,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/scheduling/priorityclass/storage/storage_test.go b/pkg/registry/scheduling/priorityclass/storage/storage_test.go index d6b8395ad72..666e6eca3da 100644 --- a/pkg/registry/scheduling/priorityclass/storage/storage_test.go +++ b/pkg/registry/scheduling/priorityclass/storage/storage_test.go @@ -40,7 +40,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "priorityclasses", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unable to create REST %v", err) + } + return rest, server } func validNewPriorityClass() *scheduling.PriorityClass { diff --git a/pkg/registry/scheduling/rest/storage_scheduling.go b/pkg/registry/scheduling/rest/storage_scheduling.go index 9d1695eb1f9..e6fe4bfc22b 100644 --- a/pkg/registry/scheduling/rest/storage_scheduling.go +++ b/pkg/registry/scheduling/rest/storage_scheduling.go @@ -46,46 +46,66 @@ type RESTStorageProvider struct{} var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(scheduling.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) if apiResourceConfigSource.VersionEnabled(schedulingapiv1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storage, err := p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1alpha1.SchemeGroupVersion.Version] = storage + } } if apiResourceConfigSource.VersionEnabled(schedulingapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storage, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1beta1.SchemeGroupVersion.Version] = storage + } } if apiResourceConfigSource.VersionEnabled(schedulingapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storage, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[schedulingapiv1.SchemeGroupVersion.Version] = storage + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // priorityclasses - priorityClassStorage := priorityclassstore.NewREST(restOptionsGetter) - storage["priorityclasses"] = priorityClassStorage - - return storage + if priorityClassStorage, err := priorityclassstore.NewREST(restOptionsGetter); err != nil { + return nil, err + } else { + storage["priorityclasses"] = priorityClassStorage + } + return storage, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // priorityclasses - priorityClassStorage := priorityclassstore.NewREST(restOptionsGetter) - storage["priorityclasses"] = priorityClassStorage + if priorityClassStorage, err := priorityclassstore.NewREST(restOptionsGetter); err != nil { + return nil, err + } else { + storage["priorityclasses"] = priorityClassStorage + } - return storage + return storage, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // priorityclasses - priorityClassStorage := priorityclassstore.NewREST(restOptionsGetter) - storage["priorityclasses"] = priorityClassStorage + if priorityClassStorage, err := priorityclassstore.NewREST(restOptionsGetter); err != nil { + return nil, err + } else { + storage["priorityclasses"] = priorityClassStorage + } - return storage + return storage, nil } func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStartHookFunc, error) { diff --git a/pkg/registry/settings/podpreset/storage/storage.go b/pkg/registry/settings/podpreset/storage/storage.go index 1bdc8c6115d..38d7b6885a7 100644 --- a/pkg/registry/settings/podpreset/storage/storage.go +++ b/pkg/registry/settings/podpreset/storage/storage.go @@ -30,7 +30,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against replication controllers. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &settingsapi.PodPreset{} }, NewListFunc: func() runtime.Object { return &settingsapi.PodPresetList{} }, @@ -42,8 +42,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } diff --git a/pkg/registry/settings/podpreset/storage/storage_test.go b/pkg/registry/settings/podpreset/storage/storage_test.go index 98239b8be49..b15cc84c0b7 100644 --- a/pkg/registry/settings/podpreset/storage/storage_test.go +++ b/pkg/registry/settings/podpreset/storage/storage_test.go @@ -39,7 +39,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "podpresets", } - return NewREST(restOptions), server + rest, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + return rest, server } func validNewPodPreset(namespace string) *settings.PodPreset { diff --git a/pkg/registry/settings/rest/storage_settings.go b/pkg/registry/settings/rest/storage_settings.go index 3f50a54f924..de21b8659ea 100644 --- a/pkg/registry/settings/rest/storage_settings.go +++ b/pkg/registry/settings/rest/storage_settings.go @@ -29,25 +29,32 @@ import ( type RESTStorageProvider struct{} -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(settings.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(settingsapiv1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[settingsapiv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, nil + } else { + apiGroupInfo.VersionedResourcesStorageMap[settingsapiv1alpha1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // podpresets - podPresetStorage := podpresetstore.NewREST(restOptionsGetter) + podPresetStorage, err := podpresetstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["podpresets"] = podPresetStorage - return storage + return storage, err } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/storage/csidriver/storage/storage.go b/pkg/registry/storage/csidriver/storage/storage.go index c22747d6f2f..c25753aade2 100644 --- a/pkg/registry/storage/csidriver/storage/storage.go +++ b/pkg/registry/storage/csidriver/storage/storage.go @@ -35,7 +35,7 @@ type REST struct { } // NewStorage returns a RESTStorage object that will work against CSIDrivers -func NewStorage(optsGetter generic.RESTOptionsGetter) *CSIDriverStorage { +func NewStorage(optsGetter generic.RESTOptionsGetter) (*CSIDriverStorage, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &storageapi.CSIDriver{} }, NewListFunc: func() runtime.Object { return &storageapi.CSIDriverList{} }, @@ -48,10 +48,10 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) *CSIDriverStorage { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } return &CSIDriverStorage{ CSIDriver: &REST{store}, - } + }, nil } diff --git a/pkg/registry/storage/csidriver/storage/storage_test.go b/pkg/registry/storage/csidriver/storage/storage_test.go index b0ad3e9c225..a1ff030c8c8 100644 --- a/pkg/registry/storage/csidriver/storage/storage_test.go +++ b/pkg/registry/storage/csidriver/storage/storage_test.go @@ -38,7 +38,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "csidrivers", } - csiDriverStorage := NewStorage(restOptions) + csiDriverStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return csiDriverStorage.CSIDriver, server } diff --git a/pkg/registry/storage/csinode/storage/storage.go b/pkg/registry/storage/csinode/storage/storage.go index ecdcb5a44b6..15e5d64af06 100644 --- a/pkg/registry/storage/csinode/storage/storage.go +++ b/pkg/registry/storage/csinode/storage/storage.go @@ -35,7 +35,7 @@ type REST struct { } // NewStorage returns a RESTStorage object that will work against CSINodes -func NewStorage(optsGetter generic.RESTOptionsGetter) *CSINodeStorage { +func NewStorage(optsGetter generic.RESTOptionsGetter) (*CSINodeStorage, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &storageapi.CSINode{} }, NewListFunc: func() runtime.Object { return &storageapi.CSINodeList{} }, @@ -48,10 +48,10 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) *CSINodeStorage { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } return &CSINodeStorage{ CSINode: &REST{store}, - } + }, nil } diff --git a/pkg/registry/storage/csinode/storage/storage_test.go b/pkg/registry/storage/csinode/storage/storage_test.go index de3075b627e..c38439b2f9d 100644 --- a/pkg/registry/storage/csinode/storage/storage_test.go +++ b/pkg/registry/storage/csinode/storage/storage_test.go @@ -38,7 +38,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "csinodes", } - csiNodeStorage := NewStorage(restOptions) + csiNodeStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return csiNodeStorage.CSINode, server } diff --git a/pkg/registry/storage/rest/storage_storage.go b/pkg/registry/storage/rest/storage_storage.go index 6cd0efbe884..9f12063c0ab 100644 --- a/pkg/registry/storage/rest/storage_storage.go +++ b/pkg/registry/storage/rest/storage_storage.go @@ -37,61 +37,94 @@ import ( type RESTStorageProvider struct { } -func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { +func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool, error) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(storageapi.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs) // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.VersionEnabled(storageapiv1alpha1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[storageapiv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[storageapiv1alpha1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(storageapiv1beta1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[storageapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[storageapiv1beta1.SchemeGroupVersion.Version] = storageMap + } } if apiResourceConfigSource.VersionEnabled(storageapiv1.SchemeGroupVersion) { - apiGroupInfo.VersionedResourcesStorageMap[storageapiv1.SchemeGroupVersion.Version] = p.v1Storage(apiResourceConfigSource, restOptionsGetter) + if storageMap, err := p.v1Storage(apiResourceConfigSource, restOptionsGetter); err != nil { + return genericapiserver.APIGroupInfo{}, false, err + } else { + apiGroupInfo.VersionedResourcesStorageMap[storageapiv1.SchemeGroupVersion.Version] = storageMap + } } - return apiGroupInfo, true + return apiGroupInfo, true, nil } -func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // volumeattachments - volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter) + volumeAttachmentStorage, err := volumeattachmentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["volumeattachments"] = volumeAttachmentStorage.VolumeAttachment - return storage + return storage, nil } -func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { +func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { storage := map[string]rest.Storage{} // storageclasses - storageClassStorage := storageclassstore.NewREST(restOptionsGetter) + storageClassStorage, err := storageclassstore.NewREST(restOptionsGetter) + if err != nil { + return storage, err + } storage["storageclasses"] = storageClassStorage // volumeattachments - volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter) + volumeAttachmentStorage, err := volumeattachmentstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["volumeattachments"] = volumeAttachmentStorage.VolumeAttachment // register csinodes if CSINodeInfo feature gate is enabled if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) { - csiNodeStorage := csinodestore.NewStorage(restOptionsGetter) + csiNodeStorage, err := csinodestore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["csinodes"] = csiNodeStorage.CSINode } // register csidrivers if CSIDriverRegistry feature gate is enabled if utilfeature.DefaultFeatureGate.Enabled(features.CSIDriverRegistry) { - csiDriverStorage := csidriverstore.NewStorage(restOptionsGetter) + csiDriverStorage, err := csidriverstore.NewStorage(restOptionsGetter) + if err != nil { + return storage, err + } storage["csidrivers"] = csiDriverStorage.CSIDriver } - return storage + return storage, nil } -func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage { - storageClassStorage := storageclassstore.NewREST(restOptionsGetter) - volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter) +func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (map[string]rest.Storage, error) { + storageClassStorage, err := storageclassstore.NewREST(restOptionsGetter) + if err != nil { + return nil, err + } + volumeAttachmentStorage, err := volumeattachmentstore.NewStorage(restOptionsGetter) + if err != nil { + return nil, err + } storage := map[string]rest.Storage{ // storageclasses @@ -102,7 +135,7 @@ func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.API "volumeattachments/status": volumeAttachmentStorage.Status, } - return storage + return storage, nil } func (p RESTStorageProvider) GroupName() string { diff --git a/pkg/registry/storage/storageclass/storage/storage.go b/pkg/registry/storage/storageclass/storage/storage.go index 11af2f3443d..40ccf276cb0 100644 --- a/pkg/registry/storage/storageclass/storage/storage.go +++ b/pkg/registry/storage/storageclass/storage/storage.go @@ -33,7 +33,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against persistent volumes. -func NewREST(optsGetter generic.RESTOptionsGetter) *REST { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &storageapi.StorageClass{} }, NewListFunc: func() runtime.Object { return &storageapi.StorageClassList{} }, @@ -48,10 +48,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } - return &REST{store} + return &REST{store}, nil } // Implement ShortNamesProvider diff --git a/pkg/registry/storage/storageclass/storage/storage_test.go b/pkg/registry/storage/storageclass/storage/storage_test.go index 6914eb52229..fb1a20dba23 100644 --- a/pkg/registry/storage/storageclass/storage/storage_test.go +++ b/pkg/registry/storage/storageclass/storage/storage_test.go @@ -39,7 +39,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "storageclasses", } - storageClassStorage := NewREST(restOptions) + storageClassStorage, err := NewREST(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return storageClassStorage, server } diff --git a/pkg/registry/storage/volumeattachment/storage/storage.go b/pkg/registry/storage/volumeattachment/storage/storage.go index 0a4643d7f89..fb74627126c 100644 --- a/pkg/registry/storage/volumeattachment/storage/storage.go +++ b/pkg/registry/storage/volumeattachment/storage/storage.go @@ -43,7 +43,7 @@ type REST struct { } // NewStorage returns a RESTStorage object that will work against VolumeAttachments -func NewStorage(optsGetter generic.RESTOptionsGetter) *VolumeAttachmentStorage { +func NewStorage(optsGetter generic.RESTOptionsGetter) (*VolumeAttachmentStorage, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &storageapi.VolumeAttachment{} }, NewListFunc: func() runtime.Object { return &storageapi.VolumeAttachmentList{} }, @@ -58,7 +58,7 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) *VolumeAttachmentStorage { } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - panic(err) // TODO: Propagate error up + return nil, err } statusStore := *store @@ -67,7 +67,7 @@ func NewStorage(optsGetter generic.RESTOptionsGetter) *VolumeAttachmentStorage { return &VolumeAttachmentStorage{ VolumeAttachment: &REST{store}, Status: &StatusREST{store: &statusStore}, - } + }, nil } // StatusREST implements the REST endpoint for changing the status of a VolumeAttachment diff --git a/pkg/registry/storage/volumeattachment/storage/storage_test.go b/pkg/registry/storage/volumeattachment/storage/storage_test.go index 4d3035c30ff..6ff5c9f749a 100644 --- a/pkg/registry/storage/volumeattachment/storage/storage_test.go +++ b/pkg/registry/storage/volumeattachment/storage/storage_test.go @@ -42,7 +42,10 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) DeleteCollectionWorkers: 1, ResourcePrefix: "volumeattachments", } - volumeAttachmentStorage := NewStorage(restOptions) + volumeAttachmentStorage, err := NewStorage(restOptions) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } return volumeAttachmentStorage.VolumeAttachment, volumeAttachmentStorage.Status, server } diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index ff3e8075002..b8600ef969b 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -97,12 +97,28 @@ func (getter *testRESTOptionsGetter) GetRESTOptions(resource schema.GroupResourc return generic.RESTOptions{StorageConfig: storageConfig, Decorator: generic.UndecoratedStorage, ResourcePrefix: resource.Resource}, nil } -func newRBACAuthorizer(config *master.Config) authorizer.Authorizer { +func newRBACAuthorizer(t *testing.T, config *master.Config) authorizer.Authorizer { optsGetter := &testRESTOptionsGetter{config} - roleRegistry := role.AuthorizerAdapter{Registry: role.NewRegistry(rolestore.NewREST(optsGetter))} - roleBindingRegistry := rolebinding.AuthorizerAdapter{Registry: rolebinding.NewRegistry(rolebindingstore.NewREST(optsGetter))} - clusterRoleRegistry := clusterrole.AuthorizerAdapter{Registry: clusterrole.NewRegistry(clusterrolestore.NewREST(optsGetter))} - clusterRoleBindingRegistry := clusterrolebinding.AuthorizerAdapter{Registry: clusterrolebinding.NewRegistry(clusterrolebindingstore.NewREST(optsGetter))} + roleRest, err := rolestore.NewREST(optsGetter) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + roleRegistry := role.AuthorizerAdapter{Registry: role.NewRegistry(roleRest)} + rolebindingRest, err := rolebindingstore.NewREST(optsGetter) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + roleBindingRegistry := rolebinding.AuthorizerAdapter{Registry: rolebinding.NewRegistry(rolebindingRest)} + clusterroleRest, err := clusterrolestore.NewREST(optsGetter) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + clusterRoleRegistry := clusterrole.AuthorizerAdapter{Registry: clusterrole.NewRegistry(clusterroleRest)} + clusterrolebindingRest, err := clusterrolebindingstore.NewREST(optsGetter) + if err != nil { + t.Fatalf("unexpected error from REST storage: %v", err) + } + clusterRoleBindingRegistry := clusterrolebinding.AuthorizerAdapter{Registry: clusterrolebinding.NewRegistry(clusterrolebindingRest)} return rbac.New(roleRegistry, roleBindingRegistry, clusterRoleRegistry, clusterRoleBindingRegistry) } @@ -515,7 +531,7 @@ func TestRBAC(t *testing.T) { for i, tc := range tests { // Create an API Server. masterConfig := framework.NewIntegrationTestMasterConfig() - masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(masterConfig) + masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(t, masterConfig) masterConfig.GenericConfig.Authentication.Authenticator = bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{ superUser: {Name: "admin", Groups: []string{"system:masters"}}, "any-rolebinding-writer": {Name: "any-rolebinding-writer"}, @@ -629,7 +645,7 @@ func TestBootstrapping(t *testing.T) { superUser := "admin/system:masters" masterConfig := framework.NewIntegrationTestMasterConfig() - masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(masterConfig) + masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(t, masterConfig) masterConfig.GenericConfig.Authentication.Authenticator = bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{ superUser: {Name: "admin", Groups: []string{"system:masters"}}, })) @@ -690,7 +706,7 @@ func TestDiscoveryUpgradeBootstrapping(t *testing.T) { superUser := "admin/system:masters" masterConfig := framework.NewIntegrationTestMasterConfig() - masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(masterConfig) + masterConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(t, masterConfig) masterConfig.GenericConfig.Authentication.Authenticator = bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{ superUser: {Name: "admin", Groups: []string{"system:masters"}}, }))