From 1fd7f35b48deef9a064726ed1de59d7eed23d189 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 00:04:53 +0100 Subject: [PATCH 1/8] Add EnvHash to instance and calculate the hash based on it --- instance/instance.pb.go | 17 ++++++++++------- protobuf/types/instance.proto | 7 +++++++ sdk/instance/instance.go | 18 ++++++++---------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/instance/instance.pb.go b/instance/instance.pb.go index 8a3357c50..30b1334e1 100644 --- a/instance/instance.pb.go +++ b/instance/instance.pb.go @@ -25,7 +25,8 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Instance represents service's instance. type Instance struct { Hash github_com_mesg_foundation_engine_hash.Hash `protobuf:"bytes,1,opt,name=hash,proto3,customtype=github.com/mesg-foundation/engine/hash.Hash" json:"hash"` - ServiceHash github_com_mesg_foundation_engine_hash.Hash `protobuf:"bytes,2,opt,name=serviceHash,proto3,customtype=github.com/mesg-foundation/engine/hash.Hash" json:"serviceHash"` + ServiceHash github_com_mesg_foundation_engine_hash.Hash `protobuf:"bytes,2,opt,name=serviceHash,proto3,customtype=github.com/mesg-foundation/engine/hash.Hash" json:"serviceHash" hash:"name:2"` + EnvHash github_com_mesg_foundation_engine_hash.Hash `protobuf:"bytes,3,opt,name=envHash,proto3,customtype=github.com/mesg-foundation/engine/hash.Hash" json:"envHash" hash:"name:3"` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` @@ -62,17 +63,19 @@ func init() { func init() { proto.RegisterFile("instance.proto", fileDescriptor_fd22322185b2070b) } var fileDescriptor_fd22322185b2070b = []byte{ - // 185 bytes of a gzipped FileDescriptorProto + // 222 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0xe2, 0xcb, 0xcc, 0x2b, 0x2e, 0x49, 0xcc, 0x4b, 0x4e, 0xd5, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0xe2, 0xca, 0x4d, 0x2d, 0x4e, 0xd7, 0x2b, 0xa9, 0x2c, 0x48, 0x2d, 0x96, 0x52, 0x4a, 0xcf, 0x4f, 0xcf, 0xd7, 0x07, 0x8b, 0x27, - 0x95, 0xa6, 0xe9, 0x83, 0x78, 0x60, 0x0e, 0x98, 0x05, 0x51, 0xaf, 0xb4, 0x8a, 0x91, 0x8b, 0xc3, + 0x95, 0xa6, 0xe9, 0x83, 0x78, 0x60, 0x0e, 0x98, 0x05, 0x51, 0xaf, 0x34, 0x9b, 0x89, 0x8b, 0xc3, 0x13, 0x6a, 0x84, 0x90, 0x3b, 0x17, 0x4b, 0x46, 0x62, 0x71, 0x86, 0x04, 0xa3, 0x02, 0xa3, 0x06, 0x8f, 0x93, 0xf1, 0x89, 0x7b, 0xf2, 0x0c, 0xb7, 0xee, 0xc9, 0x6b, 0xa7, 0x67, 0x96, 0x64, 0x94, 0x26, 0xe9, 0x25, 0xe7, 0xe7, 0xea, 0x83, 0x4c, 0xd7, 0x4d, 0xcb, 0x2f, 0xcd, 0x4b, 0x49, 0x2c, 0xc9, 0xcc, 0xcf, 0xd3, 0x4f, 0xcd, 0x4b, 0xcf, 0xcc, 0x4b, 0xd5, 0x07, 0xe9, 0xd2, 0xf3, 0x48, - 0x2c, 0xce, 0x08, 0x02, 0x1b, 0x20, 0x14, 0xca, 0xc5, 0x5d, 0x9c, 0x5a, 0x54, 0x96, 0x99, 0x9c, - 0x0a, 0x12, 0x94, 0x60, 0x22, 0xdf, 0x3c, 0x64, 0x73, 0x9c, 0x0c, 0x4e, 0x3c, 0x94, 0x63, 0x88, - 0xd2, 0x22, 0xac, 0x1f, 0x16, 0x28, 0x49, 0x6c, 0x60, 0x5f, 0x1a, 0x03, 0x02, 0x00, 0x00, 0xff, - 0xff, 0x5a, 0x2d, 0xa5, 0xc4, 0x27, 0x01, 0x00, 0x00, + 0x2c, 0xce, 0x08, 0x02, 0x1b, 0x20, 0x94, 0xc6, 0xc5, 0x5d, 0x9c, 0x5a, 0x54, 0x96, 0x99, 0x9c, + 0x0a, 0x12, 0x94, 0x60, 0x02, 0x9b, 0xe7, 0x42, 0x86, 0x79, 0x9f, 0xee, 0xc9, 0xf3, 0x82, 0x38, + 0x56, 0x4a, 0x79, 0x89, 0xb9, 0xa9, 0x56, 0x46, 0x4a, 0x41, 0xc8, 0x06, 0x0b, 0xc5, 0x71, 0xb1, + 0xa7, 0xe6, 0x95, 0x81, 0xed, 0x60, 0xa6, 0x96, 0x1d, 0xc6, 0x4a, 0x41, 0x30, 0x43, 0x9d, 0x0c, + 0x4e, 0x3c, 0x94, 0x63, 0x88, 0xd2, 0x22, 0x6c, 0x18, 0x2c, 0x16, 0x92, 0xd8, 0xc0, 0xc1, 0x6a, + 0x0c, 0x08, 0x00, 0x00, 0xff, 0xff, 0x66, 0xa1, 0x50, 0x16, 0x98, 0x01, 0x00, 0x00, } diff --git a/protobuf/types/instance.proto b/protobuf/types/instance.proto index 8e457fbe6..aadbc3bc3 100644 --- a/protobuf/types/instance.proto +++ b/protobuf/types/instance.proto @@ -15,6 +15,13 @@ message Instance { ]; bytes serviceHash = 2 [ + (gogoproto.moretags) = 'hash:"name:2"', + (gogoproto.customtype) = "github.com/mesg-foundation/engine/hash.Hash", + (gogoproto.nullable) = false + ]; + + bytes envHash = 3 [ + (gogoproto.moretags) = 'hash:"name:3"', (gogoproto.customtype) = "github.com/mesg-foundation/engine/hash.Hash", (gogoproto.nullable) = false ]; diff --git a/sdk/instance/instance.go b/sdk/instance/instance.go index 47a789a3e..94b820ab5 100644 --- a/sdk/instance/instance.go +++ b/sdk/instance/instance.go @@ -102,27 +102,25 @@ func (i *Instance) Create(serviceHash hash.Hash, env []string) (*instance.Instan if err != nil { return nil, err } + // calculate the final env vars by overwriting user defined one's with defaults. instanceEnv := xos.EnvMergeMaps(xos.EnvSliceToMap(srv.Configuration.Env), xos.EnvSliceToMap(env)) // calculate instance's hash. - h := hash.New() - h.Write(srv.Hash) - h.Write([]byte(xos.EnvMapToString(instanceEnv))) - instanceHash := h.Sum(nil) + inst := &instance.Instance{ + ServiceHash: srv.Hash, + EnvHash: hash.Dump(instanceEnv), + } + inst.Hash = hash.Dump(inst) // check if instance already exists - if exist, err := i.instanceDB.Exist(instanceHash); err != nil { + if exist, err := i.instanceDB.Exist(inst.Hash); err != nil { return nil, err } else if exist { - return nil, &AlreadyExistsError{Hash: instanceHash} + return nil, &AlreadyExistsError{Hash: inst.Hash} } // save & start instance. - inst := &instance.Instance{ - Hash: instanceHash, - ServiceHash: srv.Hash, - } if err := i.instanceDB.Save(inst); err != nil { return nil, err } From 58c1d957f6d47acb60012dcbb8a792ad1969db1f Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 07:24:01 +0100 Subject: [PATCH 2/8] Use stable sort when joining env slices --- x/xos/env.go | 3 +++ x/xos/env_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x/xos/env.go b/x/xos/env.go index d7e38e319..81c92f9cb 100644 --- a/x/xos/env.go +++ b/x/xos/env.go @@ -53,6 +53,9 @@ func EnvMergeMaps(values ...map[string]string) map[string]string { func EnvMergeSlices(values ...[]string) []string { env := make([]string, 0) for _, v := range values { + // Make sure envs are sorted to give repeatable output + // It is important for hash instance calculation + sort.Stable(sort.StringSlice(v)) env = append(env, v...) } return env diff --git a/x/xos/env_test.go b/x/xos/env_test.go index 757186b10..db5406eef 100644 --- a/x/xos/env_test.go +++ b/x/xos/env_test.go @@ -59,7 +59,7 @@ func TestEnvMergeMaps(t *testing.T) { func TestEnvMergeSlices(t *testing.T) { values := [][]string{ {"a=1", "b=2"}, - {"a=2", "c=3"}, + {"c=3", "a=2"}, } env := EnvMergeSlices(values...) for i, v := range []string{"a=1", "b=2", "a=2", "c=3"} { From d90fddc4bb56f025696d25fe3213e27abdcd18bc Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 07:59:44 +0100 Subject: [PATCH 3/8] Add error log for failed transaction --- cosmos/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cosmos/client.go b/cosmos/client.go index 17bf3d09b..02dd2f302 100644 --- a/cosmos/client.go +++ b/cosmos/client.go @@ -87,7 +87,7 @@ func (c *Client) BuildAndBroadcastMsg(msg sdktypes.Msg, accName, accPassword str return nil, errors.New("result data is not the right type") } if data.TxResult.Result.IsErr() { - return nil, errors.New("an error occurred in transaction") + return nil, fmt.Errorf("an error occurred in transaction: %s", data.TxResult.Result.Log) } return &data.TxResult.Result, nil case <-ctx.Done(): From 70a0d69274410af4d65f3f192d892845c328f9a5 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 08:01:53 +0100 Subject: [PATCH 4/8] Update validation for service configuration.env --- x/xvalidator/validator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/xvalidator/validator.go b/x/xvalidator/validator.go index e30363250..9dbef8487 100644 --- a/x/xvalidator/validator.go +++ b/x/xvalidator/validator.go @@ -41,7 +41,11 @@ func IsPortMapping(fl validator.FieldLevel) bool { } // IsEnv validates if given field is valid env variable declaration. +// The valid formats are: +// - ENV +// - ENV= +// - ENV=value func IsEnv(fl validator.FieldLevel) bool { e := strings.Split(fl.Field().String(), envSeparator) - return len(e) == 2 && envNameRegexp.MatchString(e[0]) + return (len(e) == 1 || len(e) == 2) && envNameRegexp.MatchString(e[0]) } From db4692554389920db1e96d9c1d5c4c3f5f6c5587 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 08:04:04 +0100 Subject: [PATCH 5/8] Use merge env slices with stable sort --- sdk/instance/instance.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/instance/instance.go b/sdk/instance/instance.go index 94b820ab5..b25c9e3fc 100644 --- a/sdk/instance/instance.go +++ b/sdk/instance/instance.go @@ -104,7 +104,7 @@ func (i *Instance) Create(serviceHash hash.Hash, env []string) (*instance.Instan } // calculate the final env vars by overwriting user defined one's with defaults. - instanceEnv := xos.EnvMergeMaps(xos.EnvSliceToMap(srv.Configuration.Env), xos.EnvSliceToMap(env)) + instanceEnv := xos.EnvMergeSlices(srv.Configuration.Env, env) // calculate instance's hash. inst := &instance.Instance{ @@ -125,7 +125,7 @@ func (i *Instance) Create(serviceHash hash.Hash, env []string) (*instance.Instan return nil, err } - _, err = i.start(inst, imageHash, xos.EnvMapToSlice(instanceEnv)) + _, err = i.start(inst, imageHash, instanceEnv) return inst, err } From 32a4fab59e154aeb9669f6b5487cc17f74be8f53 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 08:04:31 +0100 Subject: [PATCH 6/8] Add e2e tests for instance enviroment hash calculation --- e2e/instance_test.go | 6 +++++- e2e/testdata/test-service.json | 10 ++++++++-- e2e/testdata/test-service/mesg.yml | 7 +++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/e2e/instance_test.go b/e2e/instance_test.go index 4f02bdb42..278c8b97d 100644 --- a/e2e/instance_test.go +++ b/e2e/instance_test.go @@ -24,7 +24,10 @@ func testInstance(t *testing.T) { acknowledgement.WaitForStreamToBeReady(stream) ctx := metadata.NewOutgoingContext(context.Background(), passmd) - resp, err := client.InstanceClient.Create(ctx, &pb.CreateInstanceRequest{ServiceHash: testServiceHash}) + resp, err := client.InstanceClient.Create(ctx, &pb.CreateInstanceRequest{ + ServiceHash: testServiceHash, + Env: []string{"BAR=3", "REQUIRED=4"}, + }) require.NoError(t, err) testInstanceHash = resp.Hash @@ -38,6 +41,7 @@ func testInstance(t *testing.T) { require.NoError(t, err) require.Equal(t, testInstanceHash, resp.Hash) require.Equal(t, testServiceHash, resp.ServiceHash) + require.Equal(t, hash.Dump([]string{"BAR=2", "FOO=1", "REQUIRED", "BAR=3", "REQUIRED=4"}), resp.EnvHash) }) t.Run("list", func(t *testing.T) { diff --git a/e2e/testdata/test-service.json b/e2e/testdata/test-service.json index 5d8a5ca2f..a6cc6bc94 100644 --- a/e2e/testdata/test-service.json +++ b/e2e/testdata/test-service.json @@ -1,7 +1,13 @@ { "sid": "test-service", "name": "test-service", - "configuration": {}, + "configuration": { + "env": [ + "FOO=1", + "BAR=2", + "REQUIRED" + ] + }, "dependencies": [], "tasks": [ { @@ -85,5 +91,5 @@ ] } ], - "source": "QmNhfbBnYYTYJVFeBBPMgBtCMGKe2xXjiSdAFgDQQ3JaMz" + "source": "QmPG1Ze96pH1EgVMWsGKM33jXoG63rigMncSEqZXP7oncq" } diff --git a/e2e/testdata/test-service/mesg.yml b/e2e/testdata/test-service/mesg.yml index 07bfb5763..d63dc46bf 100644 --- a/e2e/testdata/test-service/mesg.yml +++ b/e2e/testdata/test-service/mesg.yml @@ -1,5 +1,12 @@ name: test-service sid: test-service + +configuration: + env: + - FOO=1 + - BAR=2 + - REQUIRED + tasks: ping: inputs: From 35cee55868c0acb1618b3b61aa62c8f6413ec996 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 11:05:10 +0100 Subject: [PATCH 7/8] Merge xos.Env* functions into one --- x/xos/env.go | 65 +++++++++++++---------------------------------- x/xos/env_test.go | 54 +-------------------------------------- 2 files changed, 18 insertions(+), 101 deletions(-) diff --git a/x/xos/env.go b/x/xos/env.go index 81c92f9cb..128e1be2b 100644 --- a/x/xos/env.go +++ b/x/xos/env.go @@ -5,58 +5,27 @@ import ( "strings" ) -// EnvMapToSlice transform a map of key value to a slice of env in the form "key=value". -// Env vars are sorted by names to get an accurate order while testing. -func EnvMapToSlice(values map[string]string) []string { - env := make([]string, 0, len(values)) - for k, v := range values { - env = append(env, k+"="+v) - } - sort.Stable(sort.StringSlice(env)) - return env -} - -// EnvMapToString transform a map of key value to a string in the form "key=value;key1=value1". -// Env vars are sorted by names to get an accurate order while testing. -func EnvMapToString(values map[string]string) string { - env := EnvMapToSlice(values) - return strings.Join(env, ";") -} - -// EnvSliceToMap transform a slice of key=value to a map. -func EnvSliceToMap(values []string) map[string]string { - env := make(map[string]string, len(values)) - for _, v := range values { - if e := strings.SplitN(v, "=", 2); len(e) == 1 { - env[e[0]] = "" - } else { - env[e[0]] = e[1] +// EnvMergeSlices merges multiple slices into single one. +// If the same key exist multiple time, it will be added in occurrence order. +func EnvMergeSlices(values ...[]string) []string { + envs := make(map[string]string) + for _, value := range values { + for _, v := range value { + if e := strings.SplitN(v, "=", 2); len(e) == 1 { + envs[e[0]] = "" + } else { + envs[e[0]] = e[1] + } } } - return env -} -// EnvMergeMaps merges multiple maps into single one. -// If the same key exist multiple time, it will be overwritten by the latest occurrence. -func EnvMergeMaps(values ...map[string]string) map[string]string { - env := make(map[string]string) - for _, e := range values { - for k, v := range e { - env[k] = v - } + env := make([]string, 0, len(values)) + for k, v := range envs { + env = append(env, k+"="+v) } - return env -} -// EnvMergeSlices merges multiple slices into single one. -// If the same key exist multiple time, it will be added in occurrence order. -func EnvMergeSlices(values ...[]string) []string { - env := make([]string, 0) - for _, v := range values { - // Make sure envs are sorted to give repeatable output - // It is important for hash instance calculation - sort.Stable(sort.StringSlice(v)) - env = append(env, v...) - } + // Make sure envs are sorted to give repeatable output + // It is important for hash instance calculation + sort.Stable(sort.StringSlice(env)) return env } diff --git a/x/xos/env_test.go b/x/xos/env_test.go index db5406eef..0033e0ecb 100644 --- a/x/xos/env_test.go +++ b/x/xos/env_test.go @@ -2,67 +2,15 @@ package xos import ( "testing" - - "github.com/mesg-foundation/engine/x/xstrings" ) -func TestEnvMapToSlice(t *testing.T) { - env := EnvMapToSlice(map[string]string{ - "a": "1", - "b": "2", - }) - for _, v := range []string{"a=1", "b=2"} { - if !xstrings.SliceContains(env, v) { - t.Errorf("env slice dosen't contain %s", v) - } - } -} - -func TestEnvMapToString(t *testing.T) { - got := EnvMapToString(map[string]string{ - "b": "2", - "a": "1", - }) - want := "a=1;b=2" - if got != want { - t.Errorf("invalid env string - got %s, want %s", got, want) - } -} - -func TestEnvSliceToMap(t *testing.T) { - env := EnvSliceToMap([]string{"a=1", "b=2"}) - for k, v := range map[string]string{"a": "1", "b": "2"} { - if env[k] != v { - t.Errorf("env map dosen't contain %s=%v", k, v) - } - } -} - -func TestEnvMergeMaps(t *testing.T) { - values := []map[string]string{ - { - "a": "1", - "b": "2", - }, - { - "a": "2", - "c": "3", - }, - } - env := EnvMergeMaps(values...) - for k, v := range map[string]string{"a": "2", "b": "2", "c": "3"} { - if env[k] != v { - t.Errorf("env map dosen't contain %s=%s", k, v) - } - } -} func TestEnvMergeSlices(t *testing.T) { values := [][]string{ {"a=1", "b=2"}, {"c=3", "a=2"}, } env := EnvMergeSlices(values...) - for i, v := range []string{"a=1", "b=2", "a=2", "c=3"} { + for i, v := range []string{"a=2", "b=2", "c=3"} { if env[i] != v { t.Errorf("env slice dosen't contain %s", v) } From 4c06b3f27c0a15b6544e5a8de9f5f622854ac2c6 Mon Sep 17 00:00:00 2001 From: krhubert Date: Thu, 31 Oct 2019 11:05:30 +0100 Subject: [PATCH 8/8] Fix e2e instance hash check --- e2e/instance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/instance_test.go b/e2e/instance_test.go index 278c8b97d..3a96bb349 100644 --- a/e2e/instance_test.go +++ b/e2e/instance_test.go @@ -41,7 +41,7 @@ func testInstance(t *testing.T) { require.NoError(t, err) require.Equal(t, testInstanceHash, resp.Hash) require.Equal(t, testServiceHash, resp.ServiceHash) - require.Equal(t, hash.Dump([]string{"BAR=2", "FOO=1", "REQUIRED", "BAR=3", "REQUIRED=4"}), resp.EnvHash) + require.Equal(t, hash.Dump([]string{"BAR=3", "FOO=1", "REQUIRED=4"}), resp.EnvHash) }) t.Run("list", func(t *testing.T) {