Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secret volume plugin iteration 1 #4515

Merged
merged 1 commit into from
Feb 18, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,10 @@ type GitRepo struct {
// TODO: Consider credentials here.
}

// Adapts a Secret into a VolumeSource
// Adapts a Secret into a VolumeSource.
//
// The contents of the target Secret's Data field will be presented in a volume
// as files using the keys in the Data field as the file names.
type SecretSource struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we consider SecretSource holding a slice of ObjectReferences so you could pack more than one secret into a single mount?

// Reference to a Secret
Target ObjectReference `json:"target"`
Expand Down Expand Up @@ -1318,15 +1321,22 @@ type ResourceQuotaList struct {
Items []ResourceQuota `json:"items"`
}

// Secret holds secret data of a certain type
// Secret holds secret data of a certain type. The total bytes of the values in
// the Data field must be less than MaxSecretSize bytes.
type Secret struct {
TypeMeta `json:",inline"`
ObjectMeta `json:"metadata,omitempty"`

// Data contains the secret data. Each key must be a valid DNS_SUBDOMAIN.
// The serialized form of the secret data is a base64 encoded string.
Data map[string][]byte `json:"data,omitempty"`
Type SecretType `json:"type,omitempty"`

// Used to facilitate programatic handling of secret data.
Type SecretType `json:"type,omitempty"`
}

const MaxSecretSize = 1 * 1024 * 1024

type SecretType string

const (
Expand All @@ -1339,5 +1349,3 @@ type SecretList struct {

Items []Secret `json:"items"`
}

const MaxSecretSize = 1 * 1024 * 1024
10 changes: 9 additions & 1 deletion pkg/api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1100,13 +1100,21 @@ type ResourceQuotaList struct {
Items []ResourceQuota `json:"items"`
}

// Secret holds secret data of a certain type. The total bytes of the values in
// the Data field must be less than MaxSecretSize bytes.
type Secret struct {
TypeMeta `json:",inline"`

// Data contains the secret data. Each key must be a valid DNS_SUBDOMAIN.
// The serialized form of the secret data is a base64 encoded string.
Data map[string][]byte `json:"data,omitempty"`
Type SecretType `json:"type,omitempty"`

// Used to facilitate programatic handling of secret data.
Type SecretType `json:"type,omitempty"`
}

const MaxSecretSize = 1 * 1024 * 1024

type SecretType string

const (
Expand Down
11 changes: 9 additions & 2 deletions pkg/api/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1103,14 +1103,21 @@ type ResourceQuotaList struct {
Items []ResourceQuota `json:"items"`
}

// Secret holds secret data of a certain type
// Secret holds secret data of a certain type. The total bytes of the values in
// the Data field must be less than MaxSecretSize bytes.
type Secret struct {
TypeMeta `json:",inline"`

// Data contains the secret data. Each key must be a valid DNS_SUBDOMAIN.
Data map[string][]byte `json:"data,omitempty"`
Type SecretType `json:"type,omitempty"`

// Used to facilitate programatic handling of secret data.
// The serialized form of the secret data is a base64 encoded string.
Type SecretType `json:"type,omitempty"`
}

const MaxSecretSize = 1 * 1024 * 1024

type SecretType string

const (
Expand Down
12 changes: 9 additions & 3 deletions pkg/api/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1243,16 +1243,22 @@ type ResourceQuotaList struct {
Items []ResourceQuota `json:"items"`
}

// Secret holds mappings between paths and secret data
// TODO: shouldn't "Secret" be a plural?
// Secret holds secret data of a certain type. The total bytes of the values in
// the Data field must be less than MaxSecretSize bytes.
type Secret struct {
TypeMeta `json:",inline"`
ObjectMeta `json:"metadata,omitempty"`

// Data contains the secret data. Each key must be a valid DNS_SUBDOMAIN.
// The serialized form of the secret data is a base64 encoded string.
Data map[string][]byte `json:"data,omitempty"`
Type SecretType `json:"type,omitempty"`

// Used to facilitate programatic handling of secret data.
Type SecretType `json:"type,omitempty"`
}

const MaxSecretSize = 1 * 1024 * 1024

type SecretType string

const (
Expand Down
7 changes: 6 additions & 1 deletion pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,9 +853,14 @@ func ValidateSecret(secret *api.Secret) errs.ValidationErrorList {
}

totalSize := 0
for _, value := range secret.Data {
for key, value := range secret.Data {
if !util.IsDNSSubdomain(key) {
allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("data[%v]", key), key, cIdentifierErrorMsg))
}

totalSize += len(value)
}

if totalSize > api.MaxSecretSize {
allErrs = append(allErrs, errs.NewFieldForbidden("data", "Maximum secret size exceeded"))
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2497,7 +2497,7 @@ func TestValidateSecret(t *testing.T) {
return api.Secret{
ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "bar"},
Data: map[string][]byte{
"foo": []byte("bar"),
"data-1": []byte("bar"),
},
}
}
Expand All @@ -2508,6 +2508,7 @@ func TestValidateSecret(t *testing.T) {
emptyNs = validSecret()
invalidNs = validSecret()
overMaxSize = validSecret()
invalidKey = validSecret()
)

emptyName.Name = ""
Expand All @@ -2517,6 +2518,7 @@ func TestValidateSecret(t *testing.T) {
overMaxSize.Data = map[string][]byte{
"over": make([]byte, api.MaxSecretSize+1),
}
invalidKey.Data["a..b"] = []byte("whoops")

tests := map[string]struct {
secret api.Secret
Expand All @@ -2528,6 +2530,7 @@ func TestValidateSecret(t *testing.T) {
"empty namespace": {emptyNs, false},
"invalid namespace": {invalidNs, false},
"over max size": {overMaxSize, false},
"invalid key": {invalidKey, false},
}

for name, tc := range tests {
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubelet/server/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume/gce_pd"
"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume/git_repo"
"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume/host_path"
"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume/secret"
)

// ProbeVolumePlugins collects all volume plugins into an easy to use list.
Expand All @@ -39,6 +40,7 @@ func ProbeVolumePlugins() []volume.Plugin {
allPlugins = append(allPlugins, gce_pd.ProbeVolumePlugins()...)
allPlugins = append(allPlugins, git_repo.ProbeVolumePlugins()...)
allPlugins = append(allPlugins, host_path.ProbeVolumePlugins()...)
allPlugins = append(allPlugins, secret.ProbeVolumePlugins()...)

return allPlugins
}
2 changes: 2 additions & 0 deletions pkg/kubelet/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ func (s *KubeletServer) Run(_ []string) error {
glog.Warningf("No API client: %v", err)
}

glog.Infof("Using root directory: %v", s.RootDirectory)

credentialprovider.SetPreferredDockercfgPath(s.RootDirectory)

kcfg := KubeletConfig{
Expand Down
8 changes: 4 additions & 4 deletions pkg/kubelet/volume/empty_dir/empty_dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

func TestCanSupport(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes actually necessary? Doesn't go initialize an unspecified value to nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erictune you're right, I'm not sure why I did that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erictune I remember why now; if you use field initializers you have to initialize all the fields. If you just do:

volume.FakeHost{"/tmp/fake"}

you get a too-few fields in initializer error. If you don't want to supply the nil, you have to do:

volume.FakeHost{RootDir: "/tmp/fake"}

I'm open to changing it or making a factory method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, leave it as is.


plug, err := plugMgr.FindPluginByName("kubernetes.io/empty-dir")
if err != nil {
Expand All @@ -46,7 +46,7 @@ func TestCanSupport(t *testing.T) {

func TestPlugin(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/empty-dir")
if err != nil {
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestPlugin(t *testing.T) {

func TestPluginBackCompat(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/empty-dir")
if err != nil {
Expand All @@ -125,7 +125,7 @@ func TestPluginBackCompat(t *testing.T) {

func TestPluginLegacy(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("empty")
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubelet/volume/gce_pd/gce_pd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

func TestCanSupport(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/gce-pd")
if err != nil {
Expand Down Expand Up @@ -80,7 +80,7 @@ func (fake *fakeMounter) List() ([]mount.MountPoint, error) {

func TestPlugin(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/gce-pd")
if err != nil {
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestPlugin(t *testing.T) {

func TestPluginLegacy(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil})

plug, err := plugMgr.FindPluginByName("gce-pd")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/git_repo/git_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func newTestHost(t *testing.T) volume.Host {
if err != nil {
t.Fatalf("can't make a temp rootdir: %v", err)
}
return &volume.FakeHost{tempDir}
return &volume.FakeHost{tempDir, nil}
}

func TestCanSupport(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/volume/host_path/host_path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

func TestCanSupport(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/host-path")
if err != nil {
Expand All @@ -45,7 +45,7 @@ func TestCanSupport(t *testing.T) {

func TestPlugin(t *testing.T) {
plugMgr := volume.PluginMgr{}
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"fake"})
plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"fake", nil})

plug, err := plugMgr.FindPluginByName("kubernetes.io/host-path")
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/kubelet/volume/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"sync"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors"
Expand Down Expand Up @@ -76,6 +77,9 @@ type Host interface {
// does not exist, the result of this call might not exist. This
// directory might not actually exist on disk yet.
GetPodPluginDir(podUID types.UID, pluginName string) string

// GetKubeClient returns a client interface
GetKubeClient() client.Interface
}

// PluginMgr tracks registered plugins.
Expand Down