From 37a731db798de00bf94837e207a8a418c76b6557 Mon Sep 17 00:00:00 2001
From: Fabian Ruff <fabian@progra.de>
Date: Tue, 4 Sep 2018 20:19:33 +0200
Subject: [PATCH] Avoid importing k8s.io/kubernetes from pkg/helm (#4499)
* Avoid importing k8s.io/kubernetes from pkg/helm
When writing a helm client (e.g. a helm plugin) that talks to tiller importing k8s.io/helm/pkg/helm to get the grpc client is key.
This pkg should not have a dependency to the k8s.io/kubernetes to avoid pulling in a lot of code that is only used within tiller and blow up binary sizes.
Signed-off-by: Fabian Ruff <fabian@progra.de>
* Add references to pull request in errors message
Signed-off-by: Fabian Ruff <fabian@progra.de>
* copy helper function from pkg/storage/driver
Signed-off-by: Fabian Ruff <fabian@progra.de>
* Move storage errors to seperate package
Signed-off-by: Fabian Ruff <fabian@progra.de>
* Keep old error variables for backward compatibility
Signed-off-by: Fabian Ruff <fabian@progra.de>
---
cmd/helm/upgrade.go | 4 ++--
pkg/helm/fake.go | 8 ++++----
pkg/helm/helm_test.go | 14 ++++++++++++++
pkg/storage/driver/cfgmaps.go | 9 +++++----
pkg/storage/driver/driver.go | 15 +++++++--------
pkg/storage/driver/memory.go | 15 ++++++++-------
pkg/storage/driver/records.go | 3 ++-
pkg/storage/driver/secrets.go | 9 +++++----
pkg/storage/errors/errors.go | 27 +++++++++++++++++++++++++++
9 files changed, 74 insertions(+), 30 deletions(-)
create mode 100644 pkg/storage/errors/errors.go
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index 1eeafec03..8647a2737 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -26,7 +26,7 @@ import (
"k8s.io/helm/pkg/chartutil"
"k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/renderutil"
- "k8s.io/helm/pkg/storage/driver"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
const upgradeDesc = `
@@ -207,7 +207,7 @@ func (u *upgradeCmd) run() error {
}
}
- if err != nil && strings.Contains(err.Error(), driver.ErrReleaseNotFound(u.release).Error()) {
+ if err != nil && strings.Contains(err.Error(), storageerrors.ErrReleaseNotFound(u.release).Error()) {
fmt.Fprintf(u.out, "Release %q does not exist. Installing it now.\n", u.release)
ic := &installCmd{
chartPath: chartPath,
diff --git a/pkg/helm/fake.go b/pkg/helm/fake.go
index ffb5b40c9..46be7d398 100644
--- a/pkg/helm/fake.go
+++ b/pkg/helm/fake.go
@@ -31,7 +31,7 @@ import (
rls "k8s.io/helm/pkg/proto/hapi/services"
"k8s.io/helm/pkg/proto/hapi/version"
"k8s.io/helm/pkg/renderutil"
- storage "k8s.io/helm/pkg/storage/driver"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
// FakeClient implements Interface
@@ -138,7 +138,7 @@ func (c *FakeClient) DeleteRelease(rlsName string, opts ...DeleteOption) (*rls.U
}
}
- return nil, storage.ErrReleaseNotFound(rlsName)
+ return nil, storageerrors.ErrReleaseNotFound(rlsName)
}
// GetVersion returns a fake version
@@ -212,7 +212,7 @@ func (c *FakeClient) ReleaseStatus(rlsName string, opts ...StatusOption) (*rls.G
}, nil
}
}
- return nil, storage.ErrReleaseNotFound(rlsName)
+ return nil, storageerrors.ErrReleaseNotFound(rlsName)
}
// ReleaseContent returns the configuration for the matching release name in the fake release client.
@@ -224,7 +224,7 @@ func (c *FakeClient) ReleaseContent(rlsName string, opts ...ContentOption) (resp
}, nil
}
}
- return resp, storage.ErrReleaseNotFound(rlsName)
+ return resp, storageerrors.ErrReleaseNotFound(rlsName)
}
// ReleaseHistory returns a release's revision history.
diff --git a/pkg/helm/helm_test.go b/pkg/helm/helm_test.go
index fe7150cc0..7fb794f54 100644
--- a/pkg/helm/helm_test.go
+++ b/pkg/helm/helm_test.go
@@ -18,8 +18,10 @@ package helm // import "k8s.io/helm/pkg/helm"
import (
"errors"
+ "os/exec"
"path/filepath"
"reflect"
+ "strings"
"testing"
"github.com/golang/protobuf/proto"
@@ -361,3 +363,15 @@ func loadChart(t *testing.T, name string) *cpb.Chart {
}
return c
}
+
+func TestDoesNotImportKubernetes(t *testing.T) {
+ cmd := exec.Command("go", "list", "-f", "{{.Deps}}", ".")
+ output, err := cmd.CombinedOutput()
+ if err != nil {
+ t.Fatalf("Failed to execute %s %s: %s", cmd.Path, strings.Join(cmd.Args, " "), err)
+ }
+
+ if strings.Contains(string(output), "k8s.io/kubernetes") {
+ t.Fatal("k8s.io/helm/pkg/helm contains a dependency on k8s.io/kubernetes. See https://github.com/helm/helm/pull/4499 for more details.")
+ }
+}
diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go
index 3f5ee204a..a534b67ac 100644
--- a/pkg/storage/driver/cfgmaps.go
+++ b/pkg/storage/driver/cfgmaps.go
@@ -30,6 +30,7 @@ import (
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
rspb "k8s.io/helm/pkg/proto/hapi/release"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
var _ Driver = (*ConfigMaps)(nil)
@@ -65,7 +66,7 @@ func (cfgmaps *ConfigMaps) Get(key string) (*rspb.Release, error) {
obj, err := cfgmaps.impl.Get(key, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
- return nil, ErrReleaseNotFound(key)
+ return nil, storageerrors.ErrReleaseNotFound(key)
}
cfgmaps.Log("get: failed to get %q: %s", key, err)
@@ -131,7 +132,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, err
}
if len(list.Items) == 0 {
- return nil, ErrReleaseNotFound(labels["NAME"])
+ return nil, storageerrors.ErrReleaseNotFound(labels["NAME"])
}
var results []*rspb.Release
@@ -164,7 +165,7 @@ func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error {
// push the configmap object out into the kubiverse
if _, err := cfgmaps.impl.Create(obj); err != nil {
if apierrors.IsAlreadyExists(err) {
- return ErrReleaseExists(key)
+ return storageerrors.ErrReleaseExists(key)
}
cfgmaps.Log("create: failed to create: %s", err)
@@ -202,7 +203,7 @@ func (cfgmaps *ConfigMaps) Delete(key string) (rls *rspb.Release, err error) {
// fetch the release to check existence
if rls, err = cfgmaps.Get(key); err != nil {
if apierrors.IsNotFound(err) {
- return nil, ErrReleaseExists(rls.Name)
+ return nil, storageerrors.ErrReleaseExists(rls.Name)
}
cfgmaps.Log("delete: failed to get release %q: %s", key, err)
diff --git a/pkg/storage/driver/driver.go b/pkg/storage/driver/driver.go
index d8c4122b5..3bcbd4a7e 100644
--- a/pkg/storage/driver/driver.go
+++ b/pkg/storage/driver/driver.go
@@ -17,18 +17,17 @@ limitations under the License.
package driver // import "k8s.io/helm/pkg/storage/driver"
import (
- "fmt"
-
rspb "k8s.io/helm/pkg/proto/hapi/release"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
var (
- // ErrReleaseNotFound indicates that a release is not found.
- ErrReleaseNotFound = func(release string) error { return fmt.Errorf("release: %q not found", release) }
- // ErrReleaseExists indicates that a release already exists.
- ErrReleaseExists = func(release string) error { return fmt.Errorf("release: %q already exists", release) }
- // ErrInvalidKey indicates that a release key could not be parsed.
- ErrInvalidKey = func(release string) error { return fmt.Errorf("release: %q invalid key", release) }
+ // ErrReleaseNotFound has been deprecated; please use storageerrors.ErrReleaseNotFound instead.
+ ErrReleaseNotFound = storageerrors.ErrReleaseNotFound
+ // ErrReleaseExists has been deprecated; please use storageerrors.ErrReleaseExists instead.
+ ErrReleaseExists = storageerrors.ErrReleaseExists
+ // ErrInvalidKey has been deprecated; please use storageerrors.ErrInvalidKey instead.
+ ErrInvalidKey = storageerrors.ErrInvalidKey
)
// Creator is the interface that wraps the Create method.
diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go
index ea3faf26b..700b87e08 100644
--- a/pkg/storage/driver/memory.go
+++ b/pkg/storage/driver/memory.go
@@ -22,6 +22,7 @@ import (
"sync"
rspb "k8s.io/helm/pkg/proto/hapi/release"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
var _ Driver = (*Memory)(nil)
@@ -53,16 +54,16 @@ func (mem *Memory) Get(key string) (*rspb.Release, error) {
case 2:
name, ver := elems[0], elems[1]
if _, err := strconv.Atoi(ver); err != nil {
- return nil, ErrInvalidKey(key)
+ return nil, storageerrors.ErrInvalidKey(key)
}
if recs, ok := mem.cache[name]; ok {
if r := recs.Get(key); r != nil {
return r.rls, nil
}
}
- return nil, ErrReleaseNotFound(key)
+ return nil, storageerrors.ErrReleaseNotFound(key)
default:
- return nil, ErrInvalidKey(key)
+ return nil, storageerrors.ErrInvalidKey(key)
}
}
@@ -131,7 +132,7 @@ func (mem *Memory) Update(key string, rls *rspb.Release) error {
rs.Replace(key, newRecord(key, rls))
return nil
}
- return ErrReleaseNotFound(rls.Name)
+ return storageerrors.ErrReleaseNotFound(rls.Name)
}
// Delete deletes a release or returns ErrReleaseNotFound.
@@ -141,12 +142,12 @@ func (mem *Memory) Delete(key string) (*rspb.Release, error) {
elems := strings.Split(key, ".v")
if len(elems) != 2 {
- return nil, ErrInvalidKey(key)
+ return nil, storageerrors.ErrInvalidKey(key)
}
name, ver := elems[0], elems[1]
if _, err := strconv.Atoi(ver); err != nil {
- return nil, ErrInvalidKey(key)
+ return nil, storageerrors.ErrInvalidKey(key)
}
if recs, ok := mem.cache[name]; ok {
if r := recs.Remove(key); r != nil {
@@ -155,7 +156,7 @@ func (mem *Memory) Delete(key string) (*rspb.Release, error) {
return r.rls, nil
}
}
- return nil, ErrReleaseNotFound(key)
+ return nil, storageerrors.ErrReleaseNotFound(key)
}
// wlock locks mem for writing
diff --git a/pkg/storage/driver/records.go b/pkg/storage/driver/records.go
index bc793d8b3..56d7d694f 100644
--- a/pkg/storage/driver/records.go
+++ b/pkg/storage/driver/records.go
@@ -23,6 +23,7 @@ import (
"github.com/golang/protobuf/proto"
rspb "k8s.io/helm/pkg/proto/hapi/release"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
// records holds a list of in-memory release records
@@ -38,7 +39,7 @@ func (rs *records) Add(r *record) error {
}
if rs.Exists(r.key) {
- return ErrReleaseExists(r.key)
+ return storageerrors.ErrReleaseExists(r.key)
}
*rs = append(*rs, r)
diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go
index 1b8064e53..328da20c4 100644
--- a/pkg/storage/driver/secrets.go
+++ b/pkg/storage/driver/secrets.go
@@ -30,6 +30,7 @@ import (
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
rspb "k8s.io/helm/pkg/proto/hapi/release"
+ storageerrors "k8s.io/helm/pkg/storage/errors"
)
var _ Driver = (*Secrets)(nil)
@@ -65,7 +66,7 @@ func (secrets *Secrets) Get(key string) (*rspb.Release, error) {
obj, err := secrets.impl.Get(key, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
- return nil, ErrReleaseNotFound(key)
+ return nil, storageerrors.ErrReleaseNotFound(key)
}
secrets.Log("get: failed to get %q: %s", key, err)
@@ -131,7 +132,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error)
}
if len(list.Items) == 0 {
- return nil, ErrReleaseNotFound(labels["NAME"])
+ return nil, storageerrors.ErrReleaseNotFound(labels["NAME"])
}
var results []*rspb.Release
@@ -164,7 +165,7 @@ func (secrets *Secrets) Create(key string, rls *rspb.Release) error {
// push the secret object out into the kubiverse
if _, err := secrets.impl.Create(obj); err != nil {
if apierrors.IsAlreadyExists(err) {
- return ErrReleaseExists(rls.Name)
+ return storageerrors.ErrReleaseExists(rls.Name)
}
secrets.Log("create: failed to create: %s", err)
@@ -202,7 +203,7 @@ func (secrets *Secrets) Delete(key string) (rls *rspb.Release, err error) {
// fetch the release to check existence
if rls, err = secrets.Get(key); err != nil {
if apierrors.IsNotFound(err) {
- return nil, ErrReleaseExists(rls.Name)
+ return nil, storageerrors.ErrReleaseExists(rls.Name)
}
secrets.Log("delete: failed to get release %q: %s", key, err)
diff --git a/pkg/storage/errors/errors.go b/pkg/storage/errors/errors.go
new file mode 100644
index 000000000..38662d1ca
--- /dev/null
+++ b/pkg/storage/errors/errors.go
@@ -0,0 +1,27 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package errors // import "k8s.io/helm/pkg/storage/errors"
+import "fmt"
+
+var (
+ // ErrReleaseNotFound indicates that a release is not found.
+ ErrReleaseNotFound = func(release string) error { return fmt.Errorf("release: %q not found", release) }
+ // ErrReleaseExists indicates that a release already exists.
+ ErrReleaseExists = func(release string) error { return fmt.Errorf("release: %q already exists", release) }
+ // ErrInvalidKey indicates that a release key could not be parsed.
+ ErrInvalidKey = func(release string) error { return fmt.Errorf("release: %q invalid key", release) }
+)
--
GitLab