From c5ff1a0070df43497e3f07cc27af09d1d170b7d3 Mon Sep 17 00:00:00 2001 From: Ben Stickel Date: Sat, 1 Jul 2023 18:04:42 -0700 Subject: [PATCH 1/3] test: update test to evaluate constraints per rule Signed-off-by: Ben Stickel --- pkg/bundle/ruleset/bundle_test.go | 32 +++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/bundle/ruleset/bundle_test.go b/pkg/bundle/ruleset/bundle_test.go index 50a160d9..768aac12 100644 --- a/pkg/bundle/ruleset/bundle_test.go +++ b/pkg/bundle/ruleset/bundle_test.go @@ -21,7 +21,8 @@ import ( "testing" bundlev1 "github.com/elastic/harp/api/gen/go/harp/bundle/v1" - "github.com/golang/protobuf/proto" + + "github.com/stretchr/testify/assert" ) func TestFromBundle(t *testing.T) { @@ -214,12 +215,31 @@ func TestFromBundle(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := FromBundle(tt.args.b) - if (err != nil) != tt.wantErr { - t.Errorf("error = %v, wantErr %v", err, tt.wantErr) - } + if tt.wantErr { + assert.Error(t, err) + } else { + metaOK := assert.Equal(t, got.Meta, tt.want.Meta) + if !metaOK { + t.Errorf( + "ruleset.Meta not equal. Got: %+v, Want: %+v", + got.Meta, + tt.want.Meta, + ) + } - if !proto.Equal(got, tt.want) { - t.Errorf("Ruleset not equal = %v, want %v", got, tt.want) + expectedRules := make([]*bundlev1.Rule, 0, len(tt.want.Spec.GetRules())) + for _, r := range got.Spec.GetRules() { + expectedRules = append(expectedRules, r) + } + gotRules := make([]*bundlev1.Rule, 0, len(got.Spec.GetRules())) + for _, r := range got.Spec.GetRules() { + gotRules = append(gotRules, r) + } + for idx := range expectedRules { + assert.Equal(t, expectedRules[idx].Path, gotRules[idx].Path) + assert.Equal(t, expectedRules[idx].Name, gotRules[idx].Name) + assert.Equal(t, len(expectedRules[idx].GetConstraints()), len(gotRules[idx].GetConstraints())) + } } }) } From 9c7a04882fcdd585d174a9f3b2e01e802025edac Mon Sep 17 00:00:00 2001 From: Ben Stickel Date: Sat, 1 Jul 2023 18:05:18 -0700 Subject: [PATCH 2/3] chore: mage code:lint Signed-off-by: Ben Stickel --- pkg/sdk/value/encryption/transformer_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sdk/value/encryption/transformer_test.go b/pkg/sdk/value/encryption/transformer_test.go index 854fb618..ae63ab2e 100644 --- a/pkg/sdk/value/encryption/transformer_test.go +++ b/pkg/sdk/value/encryption/transformer_test.go @@ -27,7 +27,6 @@ import ( "github.com/elastic/harp/pkg/sdk/value" "github.com/elastic/harp/pkg/sdk/value/encryption" - // Register encryption transformers _ "github.com/elastic/harp/pkg/sdk/value/encryption/aead" _ "github.com/elastic/harp/pkg/sdk/value/encryption/age" From 4804d01658d4a5adcd973ad666ccedb2205a863b Mon Sep 17 00:00:00 2001 From: Ben Stickel Date: Sun, 2 Jul 2023 23:15:43 -0700 Subject: [PATCH 3/3] test: clean-up bundle_test.go for failing tests Signed-off-by: Ben Stickel --- pkg/bundle/ruleset/bundle_test.go | 212 +++++++++++++----------------- 1 file changed, 95 insertions(+), 117 deletions(-) diff --git a/pkg/bundle/ruleset/bundle_test.go b/pkg/bundle/ruleset/bundle_test.go index 768aac12..b2f9192f 100644 --- a/pkg/bundle/ruleset/bundle_test.go +++ b/pkg/bundle/ruleset/bundle_test.go @@ -26,65 +26,56 @@ import ( ) func TestFromBundle(t *testing.T) { - type args struct { - b *bundlev1.Bundle - } tests := []struct { name string - args args + bundle *bundlev1.Bundle want *bundlev1.RuleSet wantErr bool }{ { - name: "nil", - args: args{ - b: nil, - }, + name: "nil", + bundle: &bundlev1.Bundle{}, want: nil, wantErr: true, }, { - name: "packages are nil", - args: args{ - b: &bundlev1.Bundle{ - Labels: map[string]string{ - "test": "true", - }, - Annotations: map[string]string{ - "harp.elastic.co/v1/testing#bundlePurpose": "test", - }, - Packages: nil, + name: "empty packages", + bundle: &bundlev1.Bundle{ + Labels: map[string]string{ + "test": "true", + }, + Annotations: map[string]string{ + "harp.elastic.co/v1/testing#bundlePurpose": "test", }, + Packages: nil, }, want: nil, wantErr: true, }, { name: "secrets are nil", - args: args{ - b: &bundlev1.Bundle{ - Labels: map[string]string{ - "test": "true", - }, - Annotations: map[string]string{ - "harp.elastic.co/v1/testing#bundlePurpose": "test", - }, - Packages: []*bundlev1.Package{ - { - Labels: map[string]string{ - "external": "true", - }, - Annotations: map[string]string{ - "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", - "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", - "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", - "infosec.elastic.co/v1/SecretPolicy#severity": "high", - "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", - "harp.elastic.co/v1/package#encryptionKeyAlias": "test", - }, - Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", - Secrets: nil, + bundle: &bundlev1.Bundle{ + Labels: map[string]string{ + "test": "true", + }, + Annotations: map[string]string{ + "harp.elastic.co/v1/testing#bundlePurpose": "test", + }, + Packages: []*bundlev1.Package{ + { + Labels: map[string]string{ + "external": "true", + }, + Annotations: map[string]string{ + "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", + "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", + "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", + "infosec.elastic.co/v1/SecretPolicy#severity": "high", + "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", + "harp.elastic.co/v1/package#encryptionKeyAlias": "test", }, + Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", + Secrets: nil, }, }, }, @@ -102,31 +93,29 @@ func TestFromBundle(t *testing.T) { }, { name: "secret data is nil", - args: args{ - b: &bundlev1.Bundle{ - Labels: map[string]string{ - "test": "true", - }, - Annotations: map[string]string{ - "harp.elastic.co/v1/testing#bundlePurpose": "test", - }, - Packages: []*bundlev1.Package{ - { - Labels: map[string]string{ - "external": "true", - }, - Annotations: map[string]string{ - "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", - "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", - "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", - "infosec.elastic.co/v1/SecretPolicy#severity": "high", - "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", - "harp.elastic.co/v1/package#encryptionKeyAlias": "test", - }, - Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", - Secrets: &bundlev1.SecretChain{ - Data: nil, - }, + bundle: &bundlev1.Bundle{ + Labels: map[string]string{ + "test": "true", + }, + Annotations: map[string]string{ + "harp.elastic.co/v1/testing#bundlePurpose": "test", + }, + Packages: []*bundlev1.Package{ + { + Labels: map[string]string{ + "external": "true", + }, + Annotations: map[string]string{ + "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", + "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", + "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", + "infosec.elastic.co/v1/SecretPolicy#severity": "high", + "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", + "harp.elastic.co/v1/package#encryptionKeyAlias": "test", + }, + Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", + Secrets: &bundlev1.SecretChain{ + Data: nil, }, }, }, @@ -145,38 +134,36 @@ func TestFromBundle(t *testing.T) { }, { name: "package and secrets define with annotations and labels", - args: args{ - b: &bundlev1.Bundle{ - Labels: map[string]string{ - "test": "true", - }, - Annotations: map[string]string{ - "harp.elastic.co/v1/testing#bundlePurpose": "test", - }, - Packages: []*bundlev1.Package{ - { + bundle: &bundlev1.Bundle{ + Labels: map[string]string{ + "test": "true", + }, + Annotations: map[string]string{ + "harp.elastic.co/v1/testing#bundlePurpose": "test", + }, + Packages: []*bundlev1.Package{ + { + Labels: map[string]string{ + "external": "true", + }, + Annotations: map[string]string{ + "harp.elastic.co/v1/package#encryptionKeyAlias": "test", + "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", + "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", + "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", + "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", + "infosec.elastic.co/v1/SecretPolicy#severity": "high", + }, + Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", + Secrets: &bundlev1.SecretChain{ Labels: map[string]string{ - "external": "true", - }, - Annotations: map[string]string{ - "harp.elastic.co/v1/package#encryptionKeyAlias": "test", - "infra.elastic.co/v1/CI#jobName": "rotate-external-api-key", - "infosec.elastic.co/v1/SecretPolicy#rotationMethod": "ci", - "infosec.elastic.co/v1/SecretPolicy#rotationPeriod": "90d", - "infosec.elastic.co/v1/SecretPolicy#serviceType": "authentication", - "infosec.elastic.co/v1/SecretPolicy#severity": "high", + "vendor": "true", }, - Name: "app/production/testAccount/testService/v1.0.0/internalTestComponent/authentication/api_key", - Secrets: &bundlev1.SecretChain{ - Labels: map[string]string{ - "vendor": "true", - }, - Data: []*bundlev1.KV{ - { - Key: "API_KEY", - Type: "string", - Value: []byte("3YGVuHwUqYVkjk-c6lQgfVQwFHawPG36TgAm72sPZGE="), - }, + Data: []*bundlev1.KV{ + { + Key: "API_KEY", + Type: "string", + Value: []byte("3YGVuHwUqYVkjk-c6lQgfVQwFHawPG36TgAm72sPZGE="), }, }, }, @@ -214,31 +201,22 @@ func TestFromBundle(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := FromBundle(tt.args.b) + got, err := FromBundle(tt.bundle) if tt.wantErr { assert.Error(t, err) } else { - metaOK := assert.Equal(t, got.Meta, tt.want.Meta) - if !metaOK { - t.Errorf( - "ruleset.Meta not equal. Got: %+v, Want: %+v", - got.Meta, - tt.want.Meta, - ) - } + assert.NoError(t, err) + assert.Equal(t, tt.want.ApiVersion, got.ApiVersion) + assert.Equal(t, tt.want.Kind, got.Kind) + assert.Equal(t, tt.want.Meta, got.Meta) + assert.Equal(t, len(tt.want.Spec.Rules), len(got.Spec.Rules)) - expectedRules := make([]*bundlev1.Rule, 0, len(tt.want.Spec.GetRules())) - for _, r := range got.Spec.GetRules() { - expectedRules = append(expectedRules, r) - } - gotRules := make([]*bundlev1.Rule, 0, len(got.Spec.GetRules())) - for _, r := range got.Spec.GetRules() { - gotRules = append(gotRules, r) - } - for idx := range expectedRules { - assert.Equal(t, expectedRules[idx].Path, gotRules[idx].Path) - assert.Equal(t, expectedRules[idx].Name, gotRules[idx].Name) - assert.Equal(t, len(expectedRules[idx].GetConstraints()), len(gotRules[idx].GetConstraints())) + for idx, expectedRule := range tt.want.Spec.Rules { + gotRule := got.Spec.Rules[idx] + assert.Equal(t, expectedRule.Name, gotRule.Name) + assert.Equal(t, expectedRule.Path, gotRule.Path) + assert.Equal(t, len(expectedRule.Constraints), len(gotRule.Constraints)) + assert.ElementsMatch(t, expectedRule.GetConstraints(), gotRule.GetConstraints()) } } })