From 030a81f54b29e8eb3b4e7a531d60cec4d7709fa3 Mon Sep 17 00:00:00 2001 From: Diogo Monica Date: Fri, 12 Aug 2016 17:11:27 -0700 Subject: [PATCH 1/4] Initial secret store in raft state. Cherry-picked from commit by Aaron Lehmann Signed-off-by: cyli --- manager/state/store/secrets.go | 225 +++++++++++++++++++++++++++++++++ manager/state/watch.go | 81 ++++++++++++ 2 files changed, 306 insertions(+) create mode 100644 manager/state/store/secrets.go diff --git a/manager/state/store/secrets.go b/manager/state/store/secrets.go new file mode 100644 index 0000000000..b8b25488af --- /dev/null +++ b/manager/state/store/secrets.go @@ -0,0 +1,225 @@ +package store + +import ( + "strings" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/manager/state" + memdb "github.com/hashicorp/go-memdb" +) + +const tableSecret = "secret" + +func init() { + register(ObjectStoreConfig{ + Name: tableSecret, + Table: &memdb.TableSchema{ + Name: tableSecret, + Indexes: map[string]*memdb.IndexSchema{ + indexID: { + Name: indexID, + Unique: true, + Indexer: secretIndexerByID{}, + }, + indexName: { + Name: indexName, + Unique: true, + Indexer: secretIndexerByName{}, + }, + }, + }, + Save: func(tx ReadTx, snapshot *api.StoreSnapshot) error { + var err error + snapshot.Secrets, err = FindSecrets(tx, All) + return err + }, + Restore: func(tx Tx, snapshot *api.StoreSnapshot) error { + secrets, err := FindSecrets(tx, All) + if err != nil { + return err + } + for _, s := range secrets { + if err := DeleteSecret(tx, s.ID); err != nil { + return err + } + } + for _, s := range snapshot.Secrets { + if err := CreateSecret(tx, s); err != nil { + return err + } + } + return nil + }, + ApplyStoreAction: func(tx Tx, sa *api.StoreAction) error { + switch v := sa.Target.(type) { + case *api.StoreAction_Secret: + obj := v.Secret + switch sa.Action { + case api.StoreActionKindCreate: + return CreateSecret(tx, obj) + case api.StoreActionKindUpdate: + return UpdateSecret(tx, obj) + case api.StoreActionKindRemove: + return DeleteSecret(tx, obj.ID) + } + } + return errUnknownStoreAction + }, + NewStoreAction: func(c state.Event) (api.StoreAction, error) { + var sa api.StoreAction + switch v := c.(type) { + case state.EventCreateSecret: + sa.Action = api.StoreActionKindCreate + sa.Target = &api.StoreAction_Secret{ + Secret: v.Secret, + } + case state.EventUpdateSecret: + sa.Action = api.StoreActionKindUpdate + sa.Target = &api.StoreAction_Secret{ + Secret: v.Secret, + } + case state.EventDeleteSecret: + sa.Action = api.StoreActionKindRemove + sa.Target = &api.StoreAction_Secret{ + Secret: v.Secret, + } + default: + return api.StoreAction{}, errUnknownStoreAction + } + return sa, nil + }, + }) +} + +type secretEntry struct { + *api.Secret +} + +func (s secretEntry) ID() string { + return s.Secret.ID +} + +func (s secretEntry) Meta() api.Meta { + return s.Secret.Meta +} + +func (s secretEntry) SetMeta(meta api.Meta) { + s.Secret.Meta = meta +} + +func (s secretEntry) Copy() Object { + return secretEntry{s.Secret.Copy()} +} + +func (s secretEntry) EventCreate() state.Event { + return state.EventCreateSecret{Secret: s.Secret} +} + +func (s secretEntry) EventUpdate() state.Event { + return state.EventUpdateSecret{Secret: s.Secret} +} + +func (s secretEntry) EventDelete() state.Event { + return state.EventDeleteSecret{Secret: s.Secret} +} + +// CreateSecret adds a new secret to the store. +// Returns ErrExist if the ID is already taken. +func CreateSecret(tx Tx, s *api.Secret) error { + // Ensure the name is not already in use. + if tx.lookup(tableSecret, indexName, strings.ToLower(s.Spec.Annotations.Name)) != nil { + return ErrNameConflict + } + + return tx.create(tableSecret, secretEntry{s}) +} + +// UpdateSecret updates an existing secret in the store. +// Returns ErrNotExist if the secret doesn't exist. +func UpdateSecret(tx Tx, s *api.Secret) error { + // Ensure the name is either not in use or already used by this same Secret. + if existing := tx.lookup(tableSecret, indexName, strings.ToLower(s.Spec.Annotations.Name)); existing != nil { + if existing.ID() != s.ID { + return ErrNameConflict + } + } + + return tx.update(tableSecret, secretEntry{s}) +} + +// DeleteSecret removes a secret from the store. +// Returns ErrNotExist if the secret doesn't exist. +func DeleteSecret(tx Tx, id string) error { + return tx.delete(tableSecret, id) +} + +// GetSecret looks up a secret by ID. +// Returns nil if the secret doesn't exist. +func GetSecret(tx ReadTx, id string) *api.Secret { + n := tx.get(tableSecret, id) + if n == nil { + return nil + } + return n.(secretEntry).Secret +} + +// FindSecrets selects a set of secrets and returns them. +func FindSecrets(tx ReadTx, by By) ([]*api.Secret, error) { + checkType := func(by By) error { + switch by.(type) { + case byName, byNamePrefix, byIDPrefix: + return nil + default: + return ErrInvalidFindBy + } + } + + secretList := []*api.Secret{} + appendResult := func(o Object) { + secretList = append(secretList, o.(secretEntry).Secret) + } + + err := tx.find(tableSecret, by, checkType, appendResult) + return secretList, err +} + +type secretIndexerByID struct{} + +func (ci secretIndexerByID) FromArgs(args ...interface{}) ([]byte, error) { + return fromArgs(args...) +} + +func (ci secretIndexerByID) FromObject(obj interface{}) (bool, []byte, error) { + s, ok := obj.(secretEntry) + if !ok { + panic("unexpected type passed to FromObject") + } + + // Add the null character as a terminator + val := s.Secret.ID + "\x00" + return true, []byte(val), nil +} + +func (ci secretIndexerByID) PrefixFromArgs(args ...interface{}) ([]byte, error) { + return prefixFromArgs(args...) +} + +type secretIndexerByName struct{} + +func (ci secretIndexerByName) FromArgs(args ...interface{}) ([]byte, error) { + return fromArgs(args...) +} + +func (ci secretIndexerByName) FromObject(obj interface{}) (bool, []byte, error) { + s, ok := obj.(secretEntry) + if !ok { + panic("unexpected type passed to FromObject") + } + + // Add the null character as a terminator + return true, []byte(strings.ToLower(s.Spec.Annotations.Name) + "\x00"), nil +} + +func (ci secretIndexerByName) PrefixFromArgs(args ...interface{}) ([]byte, error) { + return prefixFromArgs(args...) +} diff --git a/manager/state/watch.go b/manager/state/watch.go index 0d0a742c4d..cf1f29b5e1 100644 --- a/manager/state/watch.go +++ b/manager/state/watch.go @@ -451,6 +451,87 @@ func (e EventDeleteCluster) matches(watchEvent events.Event) bool { return true } +// SecretCheckFunc is the type of function used to perform filtering checks on +// api.Secret structures. +type SecretCheckFunc func(v1, v2 *api.Secret) bool + +// SecretCheckID is a SecretCheckFunc for matching volume IDs. +func SecretCheckID(v1, v2 *api.Secret) bool { + return v1.ID == v2.ID +} + +// EventCreateSecret is the type used to put CreateSecret events on the +// publish/subscribe queue and filter these events in calls to Watch. +type EventCreateSecret struct { + Secret *api.Secret + // Checks is a list of functions to call to filter events for a watch + // stream. They are applied with AND logic. They are only applicable for + // calls to Watch. + Checks []SecretCheckFunc +} + +func (e EventCreateSecret) matches(watchEvent events.Event) bool { + typedEvent, ok := watchEvent.(EventCreateSecret) + if !ok { + return false + } + + for _, check := range e.Checks { + if !check(e.Secret, typedEvent.Secret) { + return false + } + } + return true +} + +// EventUpdateSecret is the type used to put UpdateSecret events on the +// publish/subscribe queue and filter these events in calls to Watch. +type EventUpdateSecret struct { + Secret *api.Secret + // Checks is a list of functions to call to filter events for a watch + // stream. They are applied with AND logic. They are only applicable for + // calls to Watch. + Checks []SecretCheckFunc +} + +func (e EventUpdateSecret) matches(watchEvent events.Event) bool { + typedEvent, ok := watchEvent.(EventUpdateSecret) + if !ok { + return false + } + + for _, check := range e.Checks { + if !check(e.Secret, typedEvent.Secret) { + return false + } + } + return true +} + +// EventDeleteSecret is the type used to put DeleteSecret events on the +// publish/subscribe queue and filter these events in calls to Watch. +type EventDeleteSecret struct { + Secret *api.Secret + // Checks is a list of functions to call to filter events for a watch + // stream. They are applied with AND logic. They are only applicable for + // calls to Watch. + Checks []SecretCheckFunc +} + +func (e EventDeleteSecret) matches(watchEvent events.Event) bool { + typedEvent, ok := watchEvent.(EventDeleteSecret) + if !ok { + return false + } + + for _, check := range e.Checks { + if !check(e.Secret, typedEvent.Secret) { + return false + } + } + return true +} + // Watch takes a variable number of events to match against. The subscriber // will receive events that match any of the arguments passed to Watch. // From 445b7380a9a83644ef3f82b9861c4ac6a81ebe12 Mon Sep 17 00:00:00 2001 From: cyli Date: Wed, 31 Aug 2016 20:17:27 -0700 Subject: [PATCH 2/4] Add controlapi implementation of create, get, remove, and list secrets Signed-off-by: cyli --- api/control.pb.go | 4 - api/control.proto | 2 - api/objects.pb.go | 2 +- api/objects.proto | 2 +- manager/controlapi/secret.go | 148 +++++++++++++- manager/controlapi/secret_test.go | 322 ++++++++++++++++++++++++++++++ 6 files changed, 466 insertions(+), 14 deletions(-) create mode 100644 manager/controlapi/secret_test.go diff --git a/api/control.pb.go b/api/control.pb.go index bc5e449744..8ad69313bf 100644 --- a/api/control.pb.go +++ b/api/control.pb.go @@ -2414,8 +2414,6 @@ type ControlClient interface { // on the provided `CreateSecretRequest.SecretSpec`. // - Returns `InvalidArgument` if the `CreateSecretRequest.SecretSpec` is malformed, // or if the secret data is too long or contains invalid characters. - // - Returns `ResourceExhausted` if there are already the maximum number of allowed - // secrets in the system. // - Returns an error if the creation fails. CreateSecret(ctx context.Context, in *CreateSecretRequest, opts ...grpc.CallOption) (*CreateSecretResponse, error) // RemoveSecret removes the secret referenced by `RemoveSecretRequest.ID`. @@ -2678,8 +2676,6 @@ type ControlServer interface { // on the provided `CreateSecretRequest.SecretSpec`. // - Returns `InvalidArgument` if the `CreateSecretRequest.SecretSpec` is malformed, // or if the secret data is too long or contains invalid characters. - // - Returns `ResourceExhausted` if there are already the maximum number of allowed - // secrets in the system. // - Returns an error if the creation fails. CreateSecret(context.Context, *CreateSecretRequest) (*CreateSecretResponse, error) // RemoveSecret removes the secret referenced by `RemoveSecretRequest.ID`. diff --git a/api/control.proto b/api/control.proto index 630612916b..3be83d5600 100644 --- a/api/control.proto +++ b/api/control.proto @@ -95,8 +95,6 @@ service Control { // on the provided `CreateSecretRequest.SecretSpec`. // - Returns `InvalidArgument` if the `CreateSecretRequest.SecretSpec` is malformed, // or if the secret data is too long or contains invalid characters. - // - Returns `ResourceExhausted` if there are already the maximum number of allowed - // secrets in the system. // - Returns an error if the creation fails. rpc CreateSecret(CreateSecretRequest) returns (CreateSecretResponse) { option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" }; diff --git a/api/objects.pb.go b/api/objects.pb.go index 54c88f2254..93d515b22e 100644 --- a/api/objects.pb.go +++ b/api/objects.pb.go @@ -247,7 +247,7 @@ type Secret struct { // the form ":": for example "sha256:DEADBEEF...". It // is calculated from the data contained in `Secret.Spec.data`. Digest string `protobuf:"bytes,4,opt,name=digest,proto3" json:"digest,omitempty"` - // Size represents the size (number of bytes) of the secret data, and is + // SecretSize represents the size (number of bytes) of the secret data, and is // calculated from the data contained in `Secret.Spec.data`.. SecretSize uint32 `protobuf:"varint,5,opt,name=size,proto3" json:"size,omitempty"` // Whether the secret is an internal secret (not set by a user) or not. diff --git a/api/objects.proto b/api/objects.proto index 7317988292..56b3812a0d 100644 --- a/api/objects.proto +++ b/api/objects.proto @@ -245,7 +245,7 @@ message Secret { // is calculated from the data contained in `Secret.Spec.data`. string digest = 4; - // Size represents the size (number of bytes) of the secret data, and is + // SecretSize represents the size (number of bytes) of the secret data, and is // calculated from the data contained in `Secret.Spec.data`.. uint32 size = 5 [(gogoproto.customname) = "SecretSize"]; diff --git a/manager/controlapi/secret.go b/manager/controlapi/secret.go index a44183e324..1994a874be 100644 --- a/manager/controlapi/secret.go +++ b/manager/controlapi/secret.go @@ -1,7 +1,12 @@ package controlapi import ( + "regexp" + + "github.com/docker/distribution/digest" "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/identity" + "github.com/docker/swarmkit/manager/state/store" "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -12,13 +17,40 @@ import ( // MaxSecretSize is the maximum byte length of the `Secret.Spec.Data` field. const MaxSecretSize = 500 * 1024 // 500KB +var isValidSecretName = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-_.]{0,62}[a-zA-Z0-9])*$`) + +// assumes spec is not nil +func secretFromSecretSpec(spec *api.SecretSpec) *api.Secret { + return &api.Secret{ + ID: identity.NewID(), + Spec: *spec, + SecretSize: uint32(len(spec.Data)), + Digest: digest.FromBytes(spec.Data).String(), + } +} + // GetSecret returns a `GetSecretResponse` with a `Secret` with the same // id as `GetSecretRequest.SecretID` // - Returns `NotFound` if the Secret with the given id is not found. // - Returns `InvalidArgument` if the `GetSecretRequest.SecretID` is empty. // - Returns an error if getting fails. func (s *Server) GetSecret(ctx context.Context, request *api.GetSecretRequest) (*api.GetSecretResponse, error) { - return nil, grpc.Errorf(codes.Unimplemented, "Not yet implemented") + if request.SecretID == "" { + return nil, grpc.Errorf(codes.InvalidArgument, "secret ID must be provided") + } + + var secret *api.Secret + + s.store.View(func(tx store.ReadTx) { + secret = store.GetSecret(tx, request.SecretID) + }) + + if secret == nil { + return nil, grpc.Errorf(codes.NotFound, "secret %s not found", request.SecretID) + } + + secret.Spec.Data = nil + return &api.GetSecretResponse{Secret: secret}, nil } // ListSecrets returns a `ListSecretResponse` with a list all non-internal `Secret`s being @@ -27,18 +59,82 @@ func (s *Server) GetSecret(ctx context.Context, request *api.GetSecretRequest) ( // `ListSecretsRequest.SecretIDs`, or any id prefix in `ListSecretsRequest.IDPrefixes`. // - Returns an error if listing fails. func (s *Server) ListSecrets(ctx context.Context, request *api.ListSecretsRequest) (*api.ListSecretsResponse, error) { - return nil, grpc.Errorf(codes.Unimplemented, "Not yet implemented") + var ( + secrets []*api.Secret + respSecrets []*api.Secret + err error + byFilters []store.By + by store.By + labels map[string]string + ) + + // return all secrets that match either any of the names or any of the name prefixes (why would you give both?) + if request.Filters != nil { + for _, name := range request.Filters.Names { + byFilters = append(byFilters, store.ByName(name)) + } + for _, prefix := range request.Filters.NamePrefixes { + byFilters = append(byFilters, store.ByNamePrefix(prefix)) + } + for _, prefix := range request.Filters.IDPrefixes { + byFilters = append(byFilters, store.ByIDPrefix(prefix)) + } + labels = request.Filters.Labels + } + + switch len(byFilters) { + case 0: + by = store.All + case 1: + by = byFilters[0] + default: + by = store.Or(byFilters...) + } + + s.store.View(func(tx store.ReadTx) { + secrets, err = store.FindSecrets(tx, by) + }) + + if err != nil { + return nil, err + } + + // strip secret data from the secret, filter by label, and filter out all internal secrets + for _, secret := range secrets { + if secret.Internal || !filterMatchLabels(secret.Spec.Annotations.Labels, labels) { + continue + } + secret.Spec.Data = nil + respSecrets = append(respSecrets, secret) + } + + return &api.ListSecretsResponse{Secrets: respSecrets}, nil } // CreateSecret creates and return a `CreateSecretResponse` with a `Secret` based // on the provided `CreateSecretRequest.SecretSpec`. // - Returns `InvalidArgument` if the `CreateSecretRequest.SecretSpec` is malformed, // or if the secret data is too long or contains invalid characters. -// - Returns `ResourceExhausted` if there are already the maximum number of allowed -// secrets in the system. // - Returns an error if the creation fails. func (s *Server) CreateSecret(ctx context.Context, request *api.CreateSecretRequest) (*api.CreateSecretResponse, error) { - return nil, grpc.Errorf(codes.Unimplemented, "Not yet implemented") + if err := validateSecretSpec(request.Spec); err != nil { + return nil, err + } + + secret := secretFromSecretSpec(request.Spec) // the store will handle name conflicts + err := s.store.Update(func(tx store.Tx) error { + return store.CreateSecret(tx, secret) + }) + + switch err { + case store.ErrNameConflict: + return nil, grpc.Errorf(codes.AlreadyExists, "secret %s already exists", request.Spec.Annotations.Name) + case nil: + secret.Spec.Data = nil + return &api.CreateSecretResponse{Secret: secret}, nil + default: + return nil, err + } } // RemoveSecret removes the secret referenced by `RemoveSecretRequest.ID`. @@ -46,5 +142,45 @@ func (s *Server) CreateSecret(ctx context.Context, request *api.CreateSecretRequ // - Returns `NotFound` if the a secret named `RemoveSecretRequest.ID` is not found. // - Returns an error if the deletion fails. func (s *Server) RemoveSecret(ctx context.Context, request *api.RemoveSecretRequest) (*api.RemoveSecretResponse, error) { - return nil, grpc.Errorf(codes.Unimplemented, "Not yet implemented") + if request.SecretID == "" { + return nil, grpc.Errorf(codes.InvalidArgument, "secret ID must be provided") + } + + err := s.store.Update(func(tx store.Tx) error { + return store.DeleteSecret(tx, request.SecretID) + }) + + switch err { + case store.ErrNotExist: + return nil, grpc.Errorf(codes.NotFound, "secret %s not found", request.SecretID) + case nil: + return &api.RemoveSecretResponse{}, nil + default: + return nil, err + } +} + +func validateSecretSpec(spec *api.SecretSpec) error { + if spec == nil { + return grpc.Errorf(codes.InvalidArgument, errInvalidArgument.Error()) + } + if err := validateSecretAnnotations(spec.Annotations); err != nil { + return err + } + + if len(spec.Data) > MaxSecretSize || len(spec.Data) == 0 { + return grpc.Errorf(codes.InvalidArgument, "secret data must be between 1 and %d bytes inclusive", MaxSecretSize) + } + return nil +} + +func validateSecretAnnotations(m api.Annotations) error { + if m.Name == "" { + return grpc.Errorf(codes.InvalidArgument, "name must be provided") + } else if !isValidSecretName.MatchString(m.Name) { + // if the name doesn't match the regex + return grpc.Errorf(codes.InvalidArgument, + "invalid name, only 64 [a-zA-Z0-9-_.] characters allowed, and the start and end character must be [a-zA-Z0-9]") + } + return nil } diff --git a/manager/controlapi/secret_test.go b/manager/controlapi/secret_test.go new file mode 100644 index 0000000000..e9995afc37 --- /dev/null +++ b/manager/controlapi/secret_test.go @@ -0,0 +1,322 @@ +package controlapi + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "testing" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/manager/state/store" + "github.com/stretchr/testify/assert" + "golang.org/x/net/context" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" +) + +func createSecretSpec(name string, data []byte, labels map[string]string) *api.SecretSpec { + return &api.SecretSpec{ + Annotations: api.Annotations{Name: name, Labels: labels}, + Data: data, + } +} + +func validateChecksum(t *testing.T, secret *api.Secret, data []byte) { + checksumBytes := sha256.Sum256(data) + assert.Equal(t, "sha256:"+hex.EncodeToString(checksumBytes[:]), secret.Digest) + assert.EqualValues(t, len(data), secret.SecretSize) +} + +func TestValidateSecretSpec(t *testing.T) { + type BadServiceSpec struct { + spec *api.ServiceSpec + c codes.Code + } + + for _, badName := range []string{ + "", + ".", + "-", + "_", + ".name", + "name.", + "-name", + "name-", + "_name", + "name_", + "/a", + "a/", + "a/b", + "..", + "../a", + "a/..", + "withexclamation!", + "with space", + "with\nnewline", + "with@splat", + "with:colon", + "with;semicolon", + "snowman☃", + "o_______________________________________________________________o", // exactly 65 characters + } { + err := validateSecretSpec(createSecretSpec(badName, []byte("valid secret"), nil)) + assert.Error(t, err) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + } + + for _, badSpec := range []*api.SecretSpec{ + nil, + createSecretSpec("validName", nil, nil), + } { + err := validateSecretSpec(badSpec) + assert.Error(t, err) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + } + + for _, goodName := range []string{ + "0", + "a", + "A", + "name-with-dashes", + "name.with.dots", + "name_with_underscores", + "name.with-all_special", + "02624name035with1699numbers015125", + "o______________________________________________________________o", // exactly 64 characters + } { + err := validateSecretSpec(createSecretSpec(goodName, []byte("valid secret"), nil)) + assert.NoError(t, err) + } + + for _, good := range []*api.SecretSpec{ + createSecretSpec("validName", []byte("☃\n\t\r\x00 dg09236l;kajdgaj5%#9836[Q@!$]"), nil), + createSecretSpec("validName", []byte("valid secret"), nil), + createSecretSpec("createName", make([]byte, 1), nil), // 1 byte + } { + err := validateSecretSpec(good) + assert.NoError(t, err) + } +} + +func TestCreateSecret(t *testing.T) { + s := newTestServer(t) + + // ---- creating a secret with an invalid spec fails, thus checking that CreateSecret validates the spec ---- + _, err := s.Client.CreateSecret(context.Background(), &api.CreateSecretRequest{Spec: createSecretSpec("", nil, nil)}) + assert.Error(t, err) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + + // ---- creating a secret with a valid spec succeeds, and returns a secret that reflects the secret in the store + // exactly, but without the private data ---- + data := []byte("secret") + creationSpec := createSecretSpec("name", data, nil) + validSpecRequest := api.CreateSecretRequest{Spec: creationSpec} + + resp, err := s.Client.CreateSecret(context.Background(), &validSpecRequest) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.NotNil(t, resp.Secret) + validateChecksum(t, resp.Secret, data) + + // the data should be empty/omitted + assert.Equal(t, *createSecretSpec("name", nil, nil), resp.Secret.Spec) + + // for sanity, check that the stored secret still has the secret data + var storedSecret *api.Secret + s.Store.View(func(tx store.ReadTx) { + storedSecret = store.GetSecret(tx, resp.Secret.ID) + }) + assert.NotNil(t, storedSecret) + assert.Equal(t, data, storedSecret.Spec.Data) + + // ---- creating a secret with the same name, even if it's the exact same spec, fails due to a name conflict ---- + _, err = s.Client.CreateSecret(context.Background(), &validSpecRequest) + assert.Error(t, err) + assert.Equal(t, codes.AlreadyExists, grpc.Code(err), grpc.ErrorDesc(err)) +} + +func TestGetSecret(t *testing.T) { + s := newTestServer(t) + + // ---- getting a secret without providing an ID results in an InvalidArgument ---- + _, err := s.Client.GetSecret(context.Background(), &api.GetSecretRequest{}) + assert.Error(t, err) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + + // ---- getting a non-existent secret fails with NotFound ---- + _, err = s.Client.GetSecret(context.Background(), &api.GetSecretRequest{SecretID: "12345"}) + assert.Error(t, err) + assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + + // ---- getting an existing secret returns the secret with all the private data cleaned ---- + secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) + err = s.Store.Update(func(tx store.Tx) error { + return store.CreateSecret(tx, secret) + }) + assert.NoError(t, err) + + resp, err := s.Client.GetSecret(context.Background(), &api.GetSecretRequest{SecretID: secret.ID}) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.NotNil(t, resp.Secret) + + // the data should be empty/omitted + assert.NotEqual(t, secret, resp.Secret) + secret.Spec.Data = nil + assert.Equal(t, secret, resp.Secret) +} + +func TestRemoveSecret(t *testing.T) { + s := newTestServer(t) + + // ---- removing a secret without providing an ID results in an InvalidArgument ---- + _, err := s.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{}) + assert.Error(t, err) + assert.Equal(t, codes.InvalidArgument, grpc.Code(err), grpc.ErrorDesc(err)) + + // ---- removing a secret that exists succeeds ---- + secret := secretFromSecretSpec(createSecretSpec("name", []byte("data"), nil)) + err = s.Store.Update(func(tx store.Tx) error { + return store.CreateSecret(tx, secret) + }) + assert.NoError(t, err) + + resp, err := s.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: secret.ID}) + assert.NoError(t, err) + assert.Equal(t, api.RemoveSecretResponse{}, *resp) + + // ---- it was really removed because attempting to remove it again fails with a NotFound ---- + _, err = s.Client.RemoveSecret(context.Background(), &api.RemoveSecretRequest{SecretID: secret.ID}) + assert.Error(t, err) + assert.Equal(t, codes.NotFound, grpc.Code(err), grpc.ErrorDesc(err)) + +} + +func TestListSecrets(t *testing.T) { + s := newTestServer(t) + + listSecrets := func(req *api.ListSecretsRequest) map[string]*api.Secret { + resp, err := s.Client.ListSecrets(context.Background(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + + byName := make(map[string]*api.Secret) + for _, secret := range resp.Secrets { + byName[secret.Spec.Annotations.Name] = secret + } + return byName + } + + // ---- Listing secrets when there are no secrets returns an empty list but no error ---- + result := listSecrets(&api.ListSecretsRequest{}) + assert.Len(t, result, 0) + + // ---- Create a bunch of secrets in the store so we can test filtering ---- + allListableNames := []string{"aaa", "aab", "abc", "bbb", "bac", "bbc", "ccc", "cac", "cbc", "ddd"} + secretNamesToID := make(map[string]string) + for i, secretName := range allListableNames { + secret := secretFromSecretSpec(createSecretSpec(secretName, []byte("secret"), map[string]string{ + "mod2": fmt.Sprintf("%d", i%2), + "mod4": fmt.Sprintf("%d", i%4), + })) + err := s.Store.Update(func(tx store.Tx) error { + return store.CreateSecret(tx, secret) + }) + assert.NoError(t, err) + secretNamesToID[secretName] = secret.ID + } + // also add an internal secret to show that it's never returned + internalSecret := secretFromSecretSpec(createSecretSpec("internal", []byte("secret"), map[string]string{ + "mod2": "1", + "mod4": "1", + })) + internalSecret.Internal = true + err := s.Store.Update(func(tx store.Tx) error { + return store.CreateSecret(tx, internalSecret) + }) + assert.NoError(t, err) + secretNamesToID["internal"] = internalSecret.ID + + // ---- build up our list of expectations for what secrets get filtered ---- + + type listTestCase struct { + desc string + expected []string + filter *api.ListSecretsRequest_Filters + } + + listSecretTestCases := []listTestCase{ + { + desc: "no filter: all the available secrets are returned", + expected: allListableNames, + filter: nil, + }, + { + desc: "searching for something that doesn't match returns an empty list", + expected: nil, + filter: &api.ListSecretsRequest_Filters{Names: []string{"aa", "internal"}}, + }, + { + desc: "multiple name filters are or-ed together", + expected: []string{"aaa", "bbb", "ccc"}, + filter: &api.ListSecretsRequest_Filters{Names: []string{"aaa", "bbb", "ccc", "internal"}}, + }, + { + desc: "multiple name prefix filters are or-ed together", + expected: []string{"aaa", "aab", "bbb", "bbc"}, + filter: &api.ListSecretsRequest_Filters{NamePrefixes: []string{"aa", "bb", "int"}}, + }, + { + desc: "multiple ID prefix filters are or-ed together", + expected: []string{"aaa", "bbb"}, + filter: &api.ListSecretsRequest_Filters{IDPrefixes: []string{ + secretNamesToID["aaa"], secretNamesToID["bbb"], secretNamesToID["internal"]}, + }, + }, + { + desc: "name prefix, name, and ID prefix filters are or-ed together", + expected: []string{"aaa", "aab", "bbb", "bbc", "ccc", "ddd"}, + filter: &api.ListSecretsRequest_Filters{ + Names: []string{"aaa", "ccc", "internal"}, + NamePrefixes: []string{"aa", "bb", "int"}, + IDPrefixes: []string{secretNamesToID["aaa"], secretNamesToID["ddd"], secretNamesToID["internal"]}, + }, + }, + { + desc: "all labels in the label map must be matched", + expected: []string{allListableNames[0], allListableNames[4], allListableNames[8]}, + filter: &api.ListSecretsRequest_Filters{ + Labels: map[string]string{ + "mod2": "0", + "mod4": "0", + }, + }, + }, + { + desc: "name prefix, name, and ID prefix filters are or-ed together, but the results must match all labels in the label map", + // + indicates that these would be selected with the name/id/prefix filtering, and 0/1 at the end indicate the mod2 value: + // +"aaa"0, +"aab"1, "abc"0, +"bbb"1, "bac"0, +"bbc"1, +"ccc"0, "cac"1, "cbc"0, +"ddd"1 + expected: []string{"aaa", "ccc"}, + filter: &api.ListSecretsRequest_Filters{ + Names: []string{"aaa", "ccc", "internal"}, + NamePrefixes: []string{"aa", "bb", "int"}, + IDPrefixes: []string{secretNamesToID["aaa"], secretNamesToID["ddd"], secretNamesToID["internal"]}, + Labels: map[string]string{ + "mod2": "0", + }, + }, + }, + } + + // ---- run the filter tests ---- + + for _, expectation := range listSecretTestCases { + result := listSecrets(&api.ListSecretsRequest{Filters: expectation.filter}) + assert.Len(t, result, len(expectation.expected), expectation.desc) + for _, name := range expectation.expected { + assert.Contains(t, result, name, expectation.desc) + assert.NotNil(t, result[name], expectation.desc) + assert.Equal(t, secretNamesToID[name], result[name].ID, expectation.desc) + } + } +} From 930d873ea567a6c1ff7e2069add65c81e2de732b Mon Sep 17 00:00:00 2001 From: cyli Date: Fri, 2 Sep 2016 14:32:28 -0700 Subject: [PATCH 3/4] Add swarmctl commands to create, inspect, list and remove secrets. Signed-off-by: cyli --- cmd/swarmctl/main.go | 2 + cmd/swarmctl/secrets/cmd.go | 21 ++++++++ cmd/swarmctl/secrets/common.go | 43 ++++++++++++++++ cmd/swarmctl/secrets/create.go | 44 +++++++++++++++++ cmd/swarmctl/secrets/inspect.go | 57 +++++++++++++++++++++ cmd/swarmctl/secrets/list.go | 88 +++++++++++++++++++++++++++++++++ cmd/swarmctl/secrets/remove.go | 38 ++++++++++++++ 7 files changed, 293 insertions(+) create mode 100644 cmd/swarmctl/secrets/cmd.go create mode 100644 cmd/swarmctl/secrets/common.go create mode 100644 cmd/swarmctl/secrets/create.go create mode 100644 cmd/swarmctl/secrets/inspect.go create mode 100644 cmd/swarmctl/secrets/list.go create mode 100644 cmd/swarmctl/secrets/remove.go diff --git a/cmd/swarmctl/main.go b/cmd/swarmctl/main.go index 1a94accc31..5210830260 100644 --- a/cmd/swarmctl/main.go +++ b/cmd/swarmctl/main.go @@ -6,6 +6,7 @@ import ( "github.com/docker/swarmkit/cmd/swarmctl/cluster" "github.com/docker/swarmkit/cmd/swarmctl/network" "github.com/docker/swarmkit/cmd/swarmctl/node" + "github.com/docker/swarmkit/cmd/swarmctl/secrets" "github.com/docker/swarmkit/cmd/swarmctl/service" "github.com/docker/swarmkit/cmd/swarmctl/task" "github.com/docker/swarmkit/version" @@ -54,5 +55,6 @@ func init() { version.Cmd, network.Cmd, cluster.Cmd, + secrets.Cmd, ) } diff --git a/cmd/swarmctl/secrets/cmd.go b/cmd/swarmctl/secrets/cmd.go new file mode 100644 index 0000000000..0911e0ec34 --- /dev/null +++ b/cmd/swarmctl/secrets/cmd.go @@ -0,0 +1,21 @@ +package secrets + +import "github.com/spf13/cobra" + +var ( + // Cmd exposes the top-level service command. + Cmd = &cobra.Command{ + Use: "secret", + Aliases: nil, + Short: "Secrets management", + } +) + +func init() { + Cmd.AddCommand( + inspectCmd, + listCmd, + createCmd, + removeCmd, + ) +} diff --git a/cmd/swarmctl/secrets/common.go b/cmd/swarmctl/secrets/common.go new file mode 100644 index 0000000000..03586d853a --- /dev/null +++ b/cmd/swarmctl/secrets/common.go @@ -0,0 +1,43 @@ +package secrets + +import ( + "fmt" + + "github.com/docker/swarmkit/api" + "golang.org/x/net/context" +) + +func getSecret(ctx context.Context, c api.ControlClient, input string) (*api.Secret, error) { + // not sure what it is, match by name or id prefix + resp, err := c.ListSecrets(ctx, + &api.ListSecretsRequest{ + Filters: &api.ListSecretsRequest_Filters{ + Names: []string{input}, + IDPrefixes: []string{input}, + }, + }, + ) + if err != nil { + return nil, err + } + + switch len(resp.Secrets) { + case 0: + return nil, fmt.Errorf("secret %s not found", input) + case 1: + return resp.Secrets[0], nil + default: + // ok, multiple matches. Prefer exact ID over exact name. If no exact matches, return an error + for _, s := range resp.Secrets { + if s.ID == input { + return s, nil + } + } + for _, s := range resp.Secrets { + if s.Spec.Annotations.Name == input { + return s, nil + } + } + return nil, fmt.Errorf("secret %s is ambiguous (%d matches found)", input, len(resp.Secrets)) + } +} diff --git a/cmd/swarmctl/secrets/create.go b/cmd/swarmctl/secrets/create.go new file mode 100644 index 0000000000..bcd3ba0e46 --- /dev/null +++ b/cmd/swarmctl/secrets/create.go @@ -0,0 +1,44 @@ +package secrets + +import ( + "errors" + "fmt" + "io/ioutil" + "os" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/spf13/cobra" +) + +var createCmd = &cobra.Command{ + Use: "create ", + Short: "Create a secret", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return errors.New("create command takes a unique secret name as an argument, and accepts secret data via stdin") + } + + secretData, err := ioutil.ReadAll(os.Stdin) + if err != nil { + return fmt.Errorf("Error reading content from STDIN: %v", err) + } + + client, err := common.Dial(cmd) + if err != nil { + return err + } + + spec := &api.SecretSpec{ + Annotations: api.Annotations{Name: args[0]}, + Data: secretData, + } + + resp, err := client.CreateSecret(common.Context(cmd), &api.CreateSecretRequest{Spec: spec}) + if err != nil { + return err + } + fmt.Println(resp.Secret.ID) + return nil + }, +} diff --git a/cmd/swarmctl/secrets/inspect.go b/cmd/swarmctl/secrets/inspect.go new file mode 100644 index 0000000000..2e63e50b0a --- /dev/null +++ b/cmd/swarmctl/secrets/inspect.go @@ -0,0 +1,57 @@ +package secrets + +import ( + "errors" + "fmt" + "os" + "text/tabwriter" + "time" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/spf13/cobra" +) + +func printSecretSummary(secret *api.Secret) { + w := tabwriter.NewWriter(os.Stdout, 8, 8, 8, ' ', 0) + defer w.Flush() + + common.FprintfIfNotEmpty(w, "ID\t: %s\n", secret.ID) + common.FprintfIfNotEmpty(w, "Name\t: %s\n", secret.Spec.Annotations.Name) + if len(secret.Spec.Annotations.Labels) > 0 { + fmt.Fprintln(w, "Labels\t") + for k, v := range secret.Spec.Annotations.Labels { + fmt.Fprintf(w, " %s\t: %s\n", k, v) + } + } + common.FprintfIfNotEmpty(w, "Digest\t: %s\n", secret.Digest) + common.FprintfIfNotEmpty(w, "Size\t: %d\n", secret.SecretSize) + + created := time.Unix(int64(secret.Meta.CreatedAt.Seconds), int64(secret.Meta.CreatedAt.Nanos)) + common.FprintfIfNotEmpty(w, "Created\t: %s\n", created.Format(time.RFC822)) +} + +var ( + inspectCmd = &cobra.Command{ + Use: "inspect ", + Short: "Inspect a secret", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return errors.New("inspect command takes a single secret ID or name") + } + + client, err := common.Dial(cmd) + if err != nil { + return err + } + + secret, err := getSecret(common.Context(cmd), client, args[0]) + if err != nil { + return err + } + + printSecretSummary(secret) + return nil + }, + } +) diff --git a/cmd/swarmctl/secrets/list.go b/cmd/swarmctl/secrets/list.go new file mode 100644 index 0000000000..3367dc92d2 --- /dev/null +++ b/cmd/swarmctl/secrets/list.go @@ -0,0 +1,88 @@ +package secrets + +import ( + "errors" + "fmt" + "os" + "sort" + "text/tabwriter" + "time" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/dustin/go-humanize" + "github.com/spf13/cobra" +) + +type secretSorter []*api.Secret + +func (k secretSorter) Len() int { return len(k) } +func (k secretSorter) Swap(i, j int) { k[i], k[j] = k[j], k[i] } +func (k secretSorter) Less(i, j int) bool { + iTime := time.Unix(k[i].Meta.CreatedAt.Seconds, int64(k[i].Meta.CreatedAt.Nanos)) + jTime := time.Unix(k[j].Meta.CreatedAt.Seconds, int64(k[j].Meta.CreatedAt.Nanos)) + return jTime.Before(iTime) +} + +var ( + listCmd = &cobra.Command{ + Use: "ls", + Short: "List secrets", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return errors.New("ls command takes no arguments") + } + + flags := cmd.Flags() + quiet, err := flags.GetBool("quiet") + if err != nil { + return err + } + + client, err := common.Dial(cmd) + if err != nil { + return err + } + + resp, err := client.ListSecrets(common.Context(cmd), &api.ListSecretsRequest{}) + if err != nil { + return err + } + + var output func(*api.Secret) + + if !quiet { + w := tabwriter.NewWriter(os.Stdout, 0, 4, 4, ' ', 0) + defer func() { + // Ignore flushing errors - there's nothing we can do. + _ = w.Flush() + }() + common.PrintHeader(w, "ID", "Name", "Created", "Digest", "Size") + output = func(s *api.Secret) { + created := time.Unix(int64(s.Meta.CreatedAt.Seconds), int64(s.Meta.CreatedAt.Nanos)) + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\n", + s.ID, + s.Spec.Annotations.Name, + humanize.Time(created), + s.Digest, + s.SecretSize, + ) + } + + } else { + output = func(s *api.Secret) { fmt.Println(s.ID) } + } + + sorted := secretSorter(resp.Secrets) + sort.Sort(sorted) + for _, s := range sorted { + output(s) + } + return nil + }, + } +) + +func init() { + listCmd.Flags().BoolP("quiet", "q", false, "Only display secret names") +} diff --git a/cmd/swarmctl/secrets/remove.go b/cmd/swarmctl/secrets/remove.go new file mode 100644 index 0000000000..47cbb10720 --- /dev/null +++ b/cmd/swarmctl/secrets/remove.go @@ -0,0 +1,38 @@ +package secrets + +import ( + "errors" + "fmt" + + "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/spf13/cobra" +) + +var removeCmd = &cobra.Command{ + Use: "remove ", + Short: "Remove a secret", + Aliases: []string{"rm"}, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return errors.New("remove command takes a single secret ID or name") + } + + client, err := common.Dial(cmd) + if err != nil { + return err + } + + secret, err := getSecret(common.Context(cmd), client, args[0]) + if err != nil { + return err + } + + _, err = client.RemoveSecret(common.Context(cmd), &api.RemoveSecretRequest{SecretID: secret.ID}) + if err != nil { + return err + } + fmt.Println(secret.ID) + return nil + }, +} From aebced0f111cd563ead7e34f6362891ac9927b11 Mon Sep 17 00:00:00 2001 From: cyli Date: Mon, 26 Sep 2016 16:27:15 -0700 Subject: [PATCH 4/4] Address review comments Signed-off-by: cyli --- api/objects.pb.go | 10 +++++----- api/objects.proto | 4 ++-- cmd/swarmctl/main.go | 4 ++-- cmd/swarmctl/{secrets => secret}/cmd.go | 2 +- cmd/swarmctl/{secrets => secret}/common.go | 2 +- cmd/swarmctl/{secrets => secret}/create.go | 2 +- cmd/swarmctl/{secrets => secret}/inspect.go | 7 +++---- cmd/swarmctl/{secrets => secret}/list.go | 19 ++++++++++++++----- cmd/swarmctl/{secrets => secret}/remove.go | 2 +- manager/controlapi/secret.go | 19 ++++++++----------- manager/controlapi/secret_test.go | 11 ++++++----- 11 files changed, 44 insertions(+), 38 deletions(-) rename cmd/swarmctl/{secrets => secret}/cmd.go (94%) rename cmd/swarmctl/{secrets => secret}/common.go (98%) rename cmd/swarmctl/{secrets => secret}/create.go (98%) rename cmd/swarmctl/{secrets => secret}/inspect.go (86%) rename cmd/swarmctl/{secrets => secret}/list.go (83%) rename cmd/swarmctl/{secrets => secret}/remove.go (97%) diff --git a/api/objects.pb.go b/api/objects.pb.go index 93d515b22e..3cad399730 100644 --- a/api/objects.pb.go +++ b/api/objects.pb.go @@ -248,8 +248,8 @@ type Secret struct { // is calculated from the data contained in `Secret.Spec.data`. Digest string `protobuf:"bytes,4,opt,name=digest,proto3" json:"digest,omitempty"` // SecretSize represents the size (number of bytes) of the secret data, and is - // calculated from the data contained in `Secret.Spec.data`.. - SecretSize uint32 `protobuf:"varint,5,opt,name=size,proto3" json:"size,omitempty"` + // calculated from the data contained in `Secret.Spec.data`. + SecretSize int64 `protobuf:"varint,5,opt,name=size,proto3" json:"size,omitempty"` // Whether the secret is an internal secret (not set by a user) or not. Internal bool `protobuf:"varint,6,opt,name=internal,proto3" json:"internal,omitempty"` } @@ -3851,7 +3851,7 @@ func (m *Secret) Unmarshal(data []byte) error { } b := data[iNdEx] iNdEx++ - m.SecretSize |= (uint32(b) & 0x7F) << shift + m.SecretSize |= (int64(b) & 0x7F) << shift if b < 0x80 { break } @@ -4070,9 +4070,9 @@ var fileDescriptorObjects = []byte{ 0x36, 0x17, 0xdf, 0xa6, 0xcb, 0x52, 0x5f, 0xa6, 0x88, 0xae, 0x06, 0x34, 0xff, 0x74, 0xa0, 0x74, 0x48, 0x43, 0x41, 0xd5, 0x5b, 0x2d, 0xf8, 0xa3, 0x0b, 0x05, 0xaf, 0xe7, 0xbf, 0xc5, 0xda, 0xeb, 0x4a, 0xbd, 0xb7, 0xa0, 0x14, 0xb1, 0x01, 0x95, 0xe9, 0xd7, 0x84, 0x87, 0xad, 0x85, 0x9a, 0xe0, - 0x4a, 0xf6, 0x9a, 0x9a, 0xce, 0xaa, 0xa5, 0x0f, 0x9f, 0x55, 0x60, 0xaf, 0x29, 0x36, 0x7b, 0x68, + 0x4a, 0xf6, 0x9a, 0x9a, 0xce, 0x2a, 0xa6, 0x0f, 0x9f, 0x55, 0x60, 0xaf, 0x29, 0x36, 0x7b, 0x68, 0x1b, 0x2a, 0x2c, 0x56, 0x54, 0xc4, 0x64, 0x64, 0x32, 0xaf, 0xe0, 0x85, 0xdd, 0xd9, 0x39, 0x3d, 0xab, 0xdf, 0xf8, 0xfd, 0xac, 0x7e, 0xe3, 0x9f, 0xb3, 0xba, 0xf3, 0xfd, 0xbc, 0xee, 0x9c, 0xce, 0xeb, 0xce, 0xaf, 0xf3, 0xba, 0xf3, 0xc7, 0xbc, 0xee, 0x1c, 0x97, 0xcc, 0xbf, 0x81, 0x8f, 0xfe, - 0x0d, 0x00, 0x00, 0xff, 0xff, 0xfd, 0x7d, 0x96, 0x72, 0x7d, 0x0c, 0x00, 0x00, + 0x0d, 0x00, 0x00, 0xff, 0xff, 0x4e, 0x76, 0x6f, 0x66, 0x7d, 0x0c, 0x00, 0x00, } diff --git a/api/objects.proto b/api/objects.proto index 56b3812a0d..ab47b1a6fb 100644 --- a/api/objects.proto +++ b/api/objects.proto @@ -246,8 +246,8 @@ message Secret { string digest = 4; // SecretSize represents the size (number of bytes) of the secret data, and is - // calculated from the data contained in `Secret.Spec.data`.. - uint32 size = 5 [(gogoproto.customname) = "SecretSize"]; + // calculated from the data contained in `Secret.Spec.data`. + int64 size = 5 [(gogoproto.customname) = "SecretSize"]; // Whether the secret is an internal secret (not set by a user) or not. bool internal = 6; diff --git a/cmd/swarmctl/main.go b/cmd/swarmctl/main.go index 5210830260..afccb6c461 100644 --- a/cmd/swarmctl/main.go +++ b/cmd/swarmctl/main.go @@ -6,7 +6,7 @@ import ( "github.com/docker/swarmkit/cmd/swarmctl/cluster" "github.com/docker/swarmkit/cmd/swarmctl/network" "github.com/docker/swarmkit/cmd/swarmctl/node" - "github.com/docker/swarmkit/cmd/swarmctl/secrets" + "github.com/docker/swarmkit/cmd/swarmctl/secret" "github.com/docker/swarmkit/cmd/swarmctl/service" "github.com/docker/swarmkit/cmd/swarmctl/task" "github.com/docker/swarmkit/version" @@ -55,6 +55,6 @@ func init() { version.Cmd, network.Cmd, cluster.Cmd, - secrets.Cmd, + secret.Cmd, ) } diff --git a/cmd/swarmctl/secrets/cmd.go b/cmd/swarmctl/secret/cmd.go similarity index 94% rename from cmd/swarmctl/secrets/cmd.go rename to cmd/swarmctl/secret/cmd.go index 0911e0ec34..50f7eb72eb 100644 --- a/cmd/swarmctl/secrets/cmd.go +++ b/cmd/swarmctl/secret/cmd.go @@ -1,4 +1,4 @@ -package secrets +package secret import "github.com/spf13/cobra" diff --git a/cmd/swarmctl/secrets/common.go b/cmd/swarmctl/secret/common.go similarity index 98% rename from cmd/swarmctl/secrets/common.go rename to cmd/swarmctl/secret/common.go index 03586d853a..b6d5b0d4c3 100644 --- a/cmd/swarmctl/secrets/common.go +++ b/cmd/swarmctl/secret/common.go @@ -1,4 +1,4 @@ -package secrets +package secret import ( "fmt" diff --git a/cmd/swarmctl/secrets/create.go b/cmd/swarmctl/secret/create.go similarity index 98% rename from cmd/swarmctl/secrets/create.go rename to cmd/swarmctl/secret/create.go index bcd3ba0e46..4214d18fc1 100644 --- a/cmd/swarmctl/secrets/create.go +++ b/cmd/swarmctl/secret/create.go @@ -1,4 +1,4 @@ -package secrets +package secret import ( "errors" diff --git a/cmd/swarmctl/secrets/inspect.go b/cmd/swarmctl/secret/inspect.go similarity index 86% rename from cmd/swarmctl/secrets/inspect.go rename to cmd/swarmctl/secret/inspect.go index 2e63e50b0a..e7dc82d702 100644 --- a/cmd/swarmctl/secrets/inspect.go +++ b/cmd/swarmctl/secret/inspect.go @@ -1,14 +1,14 @@ -package secrets +package secret import ( "errors" "fmt" "os" "text/tabwriter" - "time" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/docker/swarmkit/protobuf/ptypes" "github.com/spf13/cobra" ) @@ -27,8 +27,7 @@ func printSecretSummary(secret *api.Secret) { common.FprintfIfNotEmpty(w, "Digest\t: %s\n", secret.Digest) common.FprintfIfNotEmpty(w, "Size\t: %d\n", secret.SecretSize) - created := time.Unix(int64(secret.Meta.CreatedAt.Seconds), int64(secret.Meta.CreatedAt.Nanos)) - common.FprintfIfNotEmpty(w, "Created\t: %s\n", created.Format(time.RFC822)) + common.FprintfIfNotEmpty(w, "Created\t: %s\n", ptypes.TimestampString(secret.Meta.CreatedAt)) } var ( diff --git a/cmd/swarmctl/secrets/list.go b/cmd/swarmctl/secret/list.go similarity index 83% rename from cmd/swarmctl/secrets/list.go rename to cmd/swarmctl/secret/list.go index 3367dc92d2..7565407f2d 100644 --- a/cmd/swarmctl/secrets/list.go +++ b/cmd/swarmctl/secret/list.go @@ -1,4 +1,4 @@ -package secrets +package secret import ( "errors" @@ -6,10 +6,10 @@ import ( "os" "sort" "text/tabwriter" - "time" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/cmd/swarmctl/common" + "github.com/docker/swarmkit/protobuf/ptypes" "github.com/dustin/go-humanize" "github.com/spf13/cobra" ) @@ -19,8 +19,14 @@ type secretSorter []*api.Secret func (k secretSorter) Len() int { return len(k) } func (k secretSorter) Swap(i, j int) { k[i], k[j] = k[j], k[i] } func (k secretSorter) Less(i, j int) bool { - iTime := time.Unix(k[i].Meta.CreatedAt.Seconds, int64(k[i].Meta.CreatedAt.Nanos)) - jTime := time.Unix(k[j].Meta.CreatedAt.Seconds, int64(k[j].Meta.CreatedAt.Nanos)) + iTime, err := ptypes.Timestamp(k[i].Meta.CreatedAt) + if err != nil { + panic(err) + } + jTime, err := ptypes.Timestamp(k[j].Meta.CreatedAt) + if err != nil { + panic(err) + } return jTime.Before(iTime) } @@ -59,7 +65,10 @@ var ( }() common.PrintHeader(w, "ID", "Name", "Created", "Digest", "Size") output = func(s *api.Secret) { - created := time.Unix(int64(s.Meta.CreatedAt.Seconds), int64(s.Meta.CreatedAt.Nanos)) + created, err := ptypes.Timestamp(s.Meta.CreatedAt) + if err != nil { + panic(err) + } fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\n", s.ID, s.Spec.Annotations.Name, diff --git a/cmd/swarmctl/secrets/remove.go b/cmd/swarmctl/secret/remove.go similarity index 97% rename from cmd/swarmctl/secrets/remove.go rename to cmd/swarmctl/secret/remove.go index 47cbb10720..517ae6e7d6 100644 --- a/cmd/swarmctl/secrets/remove.go +++ b/cmd/swarmctl/secret/remove.go @@ -1,4 +1,4 @@ -package secrets +package secret import ( "errors" diff --git a/manager/controlapi/secret.go b/manager/controlapi/secret.go index 1994a874be..4a2a04981e 100644 --- a/manager/controlapi/secret.go +++ b/manager/controlapi/secret.go @@ -17,14 +17,14 @@ import ( // MaxSecretSize is the maximum byte length of the `Secret.Spec.Data` field. const MaxSecretSize = 500 * 1024 // 500KB -var isValidSecretName = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-_.]{0,62}[a-zA-Z0-9])*$`) +var validSecretNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9]+(?:[a-zA-Z0-9-_.]*[a-zA-Z0-9])?$`) // assumes spec is not nil func secretFromSecretSpec(spec *api.SecretSpec) *api.Secret { return &api.Secret{ ID: identity.NewID(), Spec: *spec, - SecretSize: uint32(len(spec.Data)), + SecretSize: int64(len(spec.Data)), Digest: digest.FromBytes(spec.Data).String(), } } @@ -40,7 +40,6 @@ func (s *Server) GetSecret(ctx context.Context, request *api.GetSecretRequest) ( } var secret *api.Secret - s.store.View(func(tx store.ReadTx) { secret = store.GetSecret(tx, request.SecretID) }) @@ -49,7 +48,7 @@ func (s *Server) GetSecret(ctx context.Context, request *api.GetSecretRequest) ( return nil, grpc.Errorf(codes.NotFound, "secret %s not found", request.SecretID) } - secret.Spec.Data = nil + secret.Spec.Data = nil // clean the actual secret data so it's never returned return &api.GetSecretResponse{Secret: secret}, nil } @@ -94,7 +93,6 @@ func (s *Server) ListSecrets(ctx context.Context, request *api.ListSecretsReques s.store.View(func(tx store.ReadTx) { secrets, err = store.FindSecrets(tx, by) }) - if err != nil { return nil, err } @@ -104,7 +102,7 @@ func (s *Server) ListSecrets(ctx context.Context, request *api.ListSecretsReques if secret.Internal || !filterMatchLabels(secret.Spec.Annotations.Labels, labels) { continue } - secret.Spec.Data = nil + secret.Spec.Data = nil // clean the actual secret data so it's never returned respSecrets = append(respSecrets, secret) } @@ -130,7 +128,7 @@ func (s *Server) CreateSecret(ctx context.Context, request *api.CreateSecretRequ case store.ErrNameConflict: return nil, grpc.Errorf(codes.AlreadyExists, "secret %s already exists", request.Spec.Annotations.Name) case nil: - secret.Spec.Data = nil + secret.Spec.Data = nil // clean the actual secret data so it's never returned return &api.CreateSecretResponse{Secret: secret}, nil default: return nil, err @@ -149,7 +147,6 @@ func (s *Server) RemoveSecret(ctx context.Context, request *api.RemoveSecretRequ err := s.store.Update(func(tx store.Tx) error { return store.DeleteSecret(tx, request.SecretID) }) - switch err { case store.ErrNotExist: return nil, grpc.Errorf(codes.NotFound, "secret %s not found", request.SecretID) @@ -168,8 +165,8 @@ func validateSecretSpec(spec *api.SecretSpec) error { return err } - if len(spec.Data) > MaxSecretSize || len(spec.Data) == 0 { - return grpc.Errorf(codes.InvalidArgument, "secret data must be between 1 and %d bytes inclusive", MaxSecretSize) + if len(spec.Data) >= MaxSecretSize || len(spec.Data) < 1 { + return grpc.Errorf(codes.InvalidArgument, "secret data must be larger than 0 and less than %d bytes", MaxSecretSize) } return nil } @@ -177,7 +174,7 @@ func validateSecretSpec(spec *api.SecretSpec) error { func validateSecretAnnotations(m api.Annotations) error { if m.Name == "" { return grpc.Errorf(codes.InvalidArgument, "name must be provided") - } else if !isValidSecretName.MatchString(m.Name) { + } else if len(m.Name) > 64 || !validSecretNameRegexp.MatchString(m.Name) { // if the name doesn't match the regex return grpc.Errorf(codes.InvalidArgument, "invalid name, only 64 [a-zA-Z0-9-_.] characters allowed, and the start and end character must be [a-zA-Z0-9]") diff --git a/manager/controlapi/secret_test.go b/manager/controlapi/secret_test.go index e9995afc37..53335af3ea 100644 --- a/manager/controlapi/secret_test.go +++ b/manager/controlapi/secret_test.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "strings" "testing" "github.com/docker/swarmkit/api" @@ -57,7 +58,7 @@ func TestValidateSecretSpec(t *testing.T) { "with:colon", "with;semicolon", "snowman☃", - "o_______________________________________________________________o", // exactly 65 characters + strings.Repeat("a", 65), } { err := validateSecretSpec(createSecretSpec(badName, []byte("valid secret"), nil)) assert.Error(t, err) @@ -77,12 +78,12 @@ func TestValidateSecretSpec(t *testing.T) { "0", "a", "A", - "name-with-dashes", - "name.with.dots", - "name_with_underscores", + "name-with--dashes", + "name.with..dots", + "name_with__underscores", "name.with-all_special", "02624name035with1699numbers015125", - "o______________________________________________________________o", // exactly 64 characters + strings.Repeat("a", 64), } { err := validateSecretSpec(createSecretSpec(goodName, []byte("valid secret"), nil)) assert.NoError(t, err)