From ff9bf04c143d2ab4182e67628919419df75cde49 Mon Sep 17 00:00:00 2001 From: Ksawery Zietara Date: Fri, 11 Oct 2024 14:24:00 +0200 Subject: [PATCH 1/3] Add context to binding --- internal/broker/bind_create.go | 10 ++++++++++ internal/broker/bind_create_test.go | 4 ++++ internal/fixture/fixture.go | 6 ++++++ internal/model.go | 6 ++++++ internal/storage/dbmodel/binding.go | 2 ++ internal/storage/driver/postsql/binding.go | 16 ++++++++++++++++ internal/storage/driver/postsql/binding_test.go | 2 ++ internal/storage/postsql/write.go | 1 + .../202410111300_add_context_to_binding.down.sql | 1 + .../202410111300_add_context_to_binding.up.sql | 2 ++ 10 files changed, 50 insertions(+) create mode 100644 resources/keb/migrations/202410111300_add_context_to_binding.down.sql create mode 100644 resources/keb/migrations/202410111300_add_context_to_binding.up.sql diff --git a/internal/broker/bind_create.go b/internal/broker/bind_create.go index 5b2889ad40..6230908b49 100644 --- a/internal/broker/bind_create.go +++ b/internal/broker/bind_create.go @@ -95,6 +95,15 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d ).WithErrorKey("BindingNotSupported").Build() } + var bindingContext internal.BindingContext + if len(details.RawContext) != 0 { + err = json.Unmarshal(details.RawContext, &bindingContext) + if err != nil { + message := fmt.Sprintf("failed to unmarshal context: %s", err) + return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message) + } + } + var parameters BindingParams if len(details.RawParameters) != 0 { err = json.Unmarshal(details.RawParameters, ¶meters) @@ -127,6 +136,7 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d UpdatedAt: time.Now(), ExpirationSeconds: int64(expirationSeconds), + Context: bindingContext, } if parameters.ServiceAccount { // get kubeconfig for the instance diff --git a/internal/broker/bind_create_test.go b/internal/broker/bind_create_test.go index f0db01a22f..4d9b1b4ddf 100644 --- a/internal/broker/bind_create_test.go +++ b/internal/broker/bind_create_test.go @@ -192,6 +192,10 @@ func TestCreateBindingEndpoint(t *testing.T) { "plan_id": "%s", "parameters": { "service_account": true + }, + "context": { + "email": "john.smith@email.com", + "origin": "origin" } }`, fixture.PlanId), t) defer response.Body.Close() diff --git a/internal/fixture/fixture.go b/internal/fixture/fixture.go index 4fe317a902..1d32a39517 100644 --- a/internal/fixture/fixture.go +++ b/internal/fixture/fixture.go @@ -349,6 +349,8 @@ func FixBinding(id string) internal.Binding { } func FixBindingWithInstanceID(bindingID string, instanceID string) internal.Binding { + email := "john.smith@email.com" + origin := "origin" return internal.Binding{ ID: bindingID, InstanceID: instanceID, @@ -360,6 +362,10 @@ func FixBindingWithInstanceID(bindingID string, instanceID string) internal.Bind Kubeconfig: "kubeconfig", ExpirationSeconds: 600, BindingType: internal.BINDING_TYPE_SERVICE_ACCOUNT, + Context: internal.BindingContext{ + Email: &email, + Origin: &origin, + }, } } diff --git a/internal/model.go b/internal/model.go index 8db21cdfcd..be0466e4ef 100644 --- a/internal/model.go +++ b/internal/model.go @@ -577,4 +577,10 @@ type Binding struct { Kubeconfig string ExpirationSeconds int64 BindingType string + Context BindingContext +} + +type BindingContext struct { + Email *string `json:"email,omitempty"` + Origin *string `json:"origin,omitempty"` } diff --git a/internal/storage/dbmodel/binding.go b/internal/storage/dbmodel/binding.go index 68fbce0846..1984a50a79 100644 --- a/internal/storage/dbmodel/binding.go +++ b/internal/storage/dbmodel/binding.go @@ -1,6 +1,7 @@ package dbmodel import ( + "database/sql" "time" ) @@ -14,4 +15,5 @@ type BindingDTO struct { Kubeconfig string ExpirationSeconds int64 BindingType string + Context sql.NullString } diff --git a/internal/storage/driver/postsql/binding.go b/internal/storage/driver/postsql/binding.go index 7699424bbb..4eedcefbff 100644 --- a/internal/storage/driver/postsql/binding.go +++ b/internal/storage/driver/postsql/binding.go @@ -1,8 +1,11 @@ package postsql import ( + "encoding/json" "fmt" + "github.com/kyma-project/kyma-environment-broker/common/storage" + "github.com/kyma-project/kyma-environment-broker/internal" "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/kyma-project/kyma-environment-broker/internal/storage/dbmodel" @@ -84,6 +87,10 @@ func (s *Binding) toBindingDTO(binding *internal.Binding) (dbmodel.BindingDTO, e if err != nil { return dbmodel.BindingDTO{}, fmt.Errorf("while encrypting kubeconfig: %w", err) } + context, err := json.Marshal(binding.Context) + if err != nil { + return dbmodel.BindingDTO{}, fmt.Errorf("while marshal context: %w", err) + } return dbmodel.BindingDTO{ Kubeconfig: string(encrypted), @@ -91,6 +98,7 @@ func (s *Binding) toBindingDTO(binding *internal.Binding) (dbmodel.BindingDTO, e InstanceID: binding.InstanceID, CreatedAt: binding.CreatedAt, ExpirationSeconds: binding.ExpirationSeconds, + Context: storage.StringToSQLNullString(string(context)), }, nil } @@ -99,6 +107,13 @@ func (s *Binding) toBinding(dto dbmodel.BindingDTO) (internal.Binding, error) { if err != nil { return internal.Binding{}, fmt.Errorf("while decrypting kubeconfig: %w", err) } + context := internal.BindingContext{} + if dto.Context.Valid { + err := json.Unmarshal([]byte(dto.Context.String), &context) + if err != nil { + return internal.Binding{}, fmt.Errorf("while unmarshal context: %w", err) + } + } return internal.Binding{ Kubeconfig: string(decrypted), @@ -106,5 +121,6 @@ func (s *Binding) toBinding(dto dbmodel.BindingDTO) (internal.Binding, error) { InstanceID: dto.InstanceID, CreatedAt: dto.CreatedAt, ExpirationSeconds: dto.ExpirationSeconds, + Context: context, }, nil } diff --git a/internal/storage/driver/postsql/binding_test.go b/internal/storage/driver/postsql/binding_test.go index e784486256..3be7324033 100644 --- a/internal/storage/driver/postsql/binding_test.go +++ b/internal/storage/driver/postsql/binding_test.go @@ -40,6 +40,8 @@ func TestBinding(t *testing.T) { assert.Equal(t, fixedBinding.ExpirationSeconds, createdBinding.ExpirationSeconds) assert.NotNil(t, createdBinding.Kubeconfig) assert.Equal(t, fixedBinding.Kubeconfig, createdBinding.Kubeconfig) + assert.Equal(t, fixedBinding.Context.Email, createdBinding.Context.Email) + assert.Equal(t, fixedBinding.Context.Origin, createdBinding.Context.Origin) // when err = brokerStorage.Bindings().DeleteByBindingID(testBindingId) diff --git a/internal/storage/postsql/write.go b/internal/storage/postsql/write.go index 0d67b1116f..0307d99c06 100644 --- a/internal/storage/postsql/write.go +++ b/internal/storage/postsql/write.go @@ -41,6 +41,7 @@ func (ws writeSession) InsertBinding(binding dbmodel.BindingDTO) dberr.Error { Pair("kubeconfig", binding.Kubeconfig). Pair("expiration_seconds", binding.ExpirationSeconds). Pair("binding_type", binding.BindingType). + Pair("context", binding.Context). Exec() if err != nil { diff --git a/resources/keb/migrations/202410111300_add_context_to_binding.down.sql b/resources/keb/migrations/202410111300_add_context_to_binding.down.sql new file mode 100644 index 0000000000..e4a79bdff7 --- /dev/null +++ b/resources/keb/migrations/202410111300_add_context_to_binding.down.sql @@ -0,0 +1 @@ +ALTER TABLE bindings DROP COLUMN context; diff --git a/resources/keb/migrations/202410111300_add_context_to_binding.up.sql b/resources/keb/migrations/202410111300_add_context_to_binding.up.sql new file mode 100644 index 0000000000..73877f997e --- /dev/null +++ b/resources/keb/migrations/202410111300_add_context_to_binding.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE bindings + ADD COLUMN context json; From d254809dc24c2db2e7291e984f9c7619cc2db93c Mon Sep 17 00:00:00 2001 From: Ksawery Zietara Date: Fri, 11 Oct 2024 15:34:02 +0200 Subject: [PATCH 2/3] Rename context field to created_by --- internal/broker/bind_create.go | 31 +++++++++++++++- internal/broker/bind_create_test.go | 37 +++++++++++++++++++ internal/fixture/fixture.go | 7 +--- internal/model.go | 7 +--- internal/storage/dbmodel/binding.go | 3 +- internal/storage/driver/postsql/binding.go | 18 +-------- .../storage/driver/postsql/binding_test.go | 3 +- internal/storage/postsql/write.go | 2 +- ...2410111300_add_context_to_binding.down.sql | 1 - ...202410111300_add_context_to_binding.up.sql | 2 - ...0111300_add_created_by_to_binding.down.sql | 1 + ...410111300_add_created_by_to_binding.up.sql | 2 + 12 files changed, 76 insertions(+), 38 deletions(-) delete mode 100644 resources/keb/migrations/202410111300_add_context_to_binding.down.sql delete mode 100644 resources/keb/migrations/202410111300_add_context_to_binding.up.sql create mode 100644 resources/keb/migrations/202410111300_add_created_by_to_binding.down.sql create mode 100644 resources/keb/migrations/202410111300_add_created_by_to_binding.up.sql diff --git a/internal/broker/bind_create.go b/internal/broker/bind_create.go index 6230908b49..9fa3b34171 100644 --- a/internal/broker/bind_create.go +++ b/internal/broker/bind_create.go @@ -39,6 +39,33 @@ type BindEndpoint struct { log logrus.FieldLogger } +type BindingContext struct { + Email *string `json:"email,omitempty"` + Origin *string `json:"origin,omitempty"` +} + +func (b *BindingContext) CreatedBy() string { + if b.Email == nil && b.Origin == nil { + return "" + } + + email := "" + if b.Email != nil { + email = *b.Email + } + + origin := "" + if b.Origin != nil { + origin = *b.Origin + } + + if email != "" && origin != "" { + return email + " " + origin + } + + return email + origin +} + type BindingParams struct { ServiceAccount bool `json:"service_account,omit"` ExpirationSeconds int `json:"expiration_seconds,omit"` @@ -95,7 +122,7 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d ).WithErrorKey("BindingNotSupported").Build() } - var bindingContext internal.BindingContext + var bindingContext BindingContext if len(details.RawContext) != 0 { err = json.Unmarshal(details.RawContext, &bindingContext) if err != nil { @@ -136,7 +163,7 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d UpdatedAt: time.Now(), ExpirationSeconds: int64(expirationSeconds), - Context: bindingContext, + CreatedBy: bindingContext.CreatedBy(), } if parameters.ServiceAccount { // get kubeconfig for the instance diff --git a/internal/broker/bind_create_test.go b/internal/broker/bind_create_test.go index 4d9b1b4ddf..165a8eb965 100644 --- a/internal/broker/bind_create_test.go +++ b/internal/broker/bind_create_test.go @@ -384,6 +384,43 @@ func TestCreateBindingEndpoint(t *testing.T) { }) } +func TestCreatedBy(t *testing.T) { + email := "john.smith@email.com" + origin := "origin" + tests := []struct { + name string + context BindingContext + expected string + }{ + { + name: "Both Email and Origin are nil", + context: BindingContext{Email: nil, Origin: nil}, + expected: "", + }, + { + name: "Only Email is set", + context: BindingContext{Email: &email, Origin: nil}, + expected: "john.smith@email.com", + }, + { + name: "Only Origin is set", + context: BindingContext{Email: nil, Origin: &origin}, + expected: "origin", + }, + { + name: "Both Email and Origin are set", + context: BindingContext{Email: &email, Origin: &origin}, + expected: "john.smith@email.com origin", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.context.CreatedBy()) + }) + } +} + func assertClusterAccess(t *testing.T, response *http.Response, controlSecretName string, binding domain.Binding) { credentials, ok := binding.Credentials.(map[string]interface{}) diff --git a/internal/fixture/fixture.go b/internal/fixture/fixture.go index 1d32a39517..fc0ed67fbc 100644 --- a/internal/fixture/fixture.go +++ b/internal/fixture/fixture.go @@ -349,8 +349,6 @@ func FixBinding(id string) internal.Binding { } func FixBindingWithInstanceID(bindingID string, instanceID string) internal.Binding { - email := "john.smith@email.com" - origin := "origin" return internal.Binding{ ID: bindingID, InstanceID: instanceID, @@ -362,10 +360,7 @@ func FixBindingWithInstanceID(bindingID string, instanceID string) internal.Bind Kubeconfig: "kubeconfig", ExpirationSeconds: 600, BindingType: internal.BINDING_TYPE_SERVICE_ACCOUNT, - Context: internal.BindingContext{ - Email: &email, - Origin: &origin, - }, + CreatedBy: "john.smith@email.com", } } diff --git a/internal/model.go b/internal/model.go index be0466e4ef..6922c3d3d4 100644 --- a/internal/model.go +++ b/internal/model.go @@ -577,10 +577,5 @@ type Binding struct { Kubeconfig string ExpirationSeconds int64 BindingType string - Context BindingContext -} - -type BindingContext struct { - Email *string `json:"email,omitempty"` - Origin *string `json:"origin,omitempty"` + CreatedBy string } diff --git a/internal/storage/dbmodel/binding.go b/internal/storage/dbmodel/binding.go index 1984a50a79..8f8f7a6aa5 100644 --- a/internal/storage/dbmodel/binding.go +++ b/internal/storage/dbmodel/binding.go @@ -1,7 +1,6 @@ package dbmodel import ( - "database/sql" "time" ) @@ -15,5 +14,5 @@ type BindingDTO struct { Kubeconfig string ExpirationSeconds int64 BindingType string - Context sql.NullString + CreatedBy string } diff --git a/internal/storage/driver/postsql/binding.go b/internal/storage/driver/postsql/binding.go index 4eedcefbff..53fb0978ae 100644 --- a/internal/storage/driver/postsql/binding.go +++ b/internal/storage/driver/postsql/binding.go @@ -1,11 +1,8 @@ package postsql import ( - "encoding/json" "fmt" - "github.com/kyma-project/kyma-environment-broker/common/storage" - "github.com/kyma-project/kyma-environment-broker/internal" "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/kyma-project/kyma-environment-broker/internal/storage/dbmodel" @@ -87,10 +84,6 @@ func (s *Binding) toBindingDTO(binding *internal.Binding) (dbmodel.BindingDTO, e if err != nil { return dbmodel.BindingDTO{}, fmt.Errorf("while encrypting kubeconfig: %w", err) } - context, err := json.Marshal(binding.Context) - if err != nil { - return dbmodel.BindingDTO{}, fmt.Errorf("while marshal context: %w", err) - } return dbmodel.BindingDTO{ Kubeconfig: string(encrypted), @@ -98,7 +91,7 @@ func (s *Binding) toBindingDTO(binding *internal.Binding) (dbmodel.BindingDTO, e InstanceID: binding.InstanceID, CreatedAt: binding.CreatedAt, ExpirationSeconds: binding.ExpirationSeconds, - Context: storage.StringToSQLNullString(string(context)), + CreatedBy: binding.CreatedBy, }, nil } @@ -107,13 +100,6 @@ func (s *Binding) toBinding(dto dbmodel.BindingDTO) (internal.Binding, error) { if err != nil { return internal.Binding{}, fmt.Errorf("while decrypting kubeconfig: %w", err) } - context := internal.BindingContext{} - if dto.Context.Valid { - err := json.Unmarshal([]byte(dto.Context.String), &context) - if err != nil { - return internal.Binding{}, fmt.Errorf("while unmarshal context: %w", err) - } - } return internal.Binding{ Kubeconfig: string(decrypted), @@ -121,6 +107,6 @@ func (s *Binding) toBinding(dto dbmodel.BindingDTO) (internal.Binding, error) { InstanceID: dto.InstanceID, CreatedAt: dto.CreatedAt, ExpirationSeconds: dto.ExpirationSeconds, - Context: context, + CreatedBy: dto.CreatedBy, }, nil } diff --git a/internal/storage/driver/postsql/binding_test.go b/internal/storage/driver/postsql/binding_test.go index 3be7324033..f4094221fa 100644 --- a/internal/storage/driver/postsql/binding_test.go +++ b/internal/storage/driver/postsql/binding_test.go @@ -40,8 +40,7 @@ func TestBinding(t *testing.T) { assert.Equal(t, fixedBinding.ExpirationSeconds, createdBinding.ExpirationSeconds) assert.NotNil(t, createdBinding.Kubeconfig) assert.Equal(t, fixedBinding.Kubeconfig, createdBinding.Kubeconfig) - assert.Equal(t, fixedBinding.Context.Email, createdBinding.Context.Email) - assert.Equal(t, fixedBinding.Context.Origin, createdBinding.Context.Origin) + assert.Equal(t, fixedBinding.CreatedBy, createdBinding.CreatedBy) // when err = brokerStorage.Bindings().DeleteByBindingID(testBindingId) diff --git a/internal/storage/postsql/write.go b/internal/storage/postsql/write.go index 0307d99c06..5a5f9b337b 100644 --- a/internal/storage/postsql/write.go +++ b/internal/storage/postsql/write.go @@ -41,7 +41,7 @@ func (ws writeSession) InsertBinding(binding dbmodel.BindingDTO) dberr.Error { Pair("kubeconfig", binding.Kubeconfig). Pair("expiration_seconds", binding.ExpirationSeconds). Pair("binding_type", binding.BindingType). - Pair("context", binding.Context). + Pair("created_by", binding.CreatedBy). Exec() if err != nil { diff --git a/resources/keb/migrations/202410111300_add_context_to_binding.down.sql b/resources/keb/migrations/202410111300_add_context_to_binding.down.sql deleted file mode 100644 index e4a79bdff7..0000000000 --- a/resources/keb/migrations/202410111300_add_context_to_binding.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE bindings DROP COLUMN context; diff --git a/resources/keb/migrations/202410111300_add_context_to_binding.up.sql b/resources/keb/migrations/202410111300_add_context_to_binding.up.sql deleted file mode 100644 index 73877f997e..0000000000 --- a/resources/keb/migrations/202410111300_add_context_to_binding.up.sql +++ /dev/null @@ -1,2 +0,0 @@ -ALTER TABLE bindings - ADD COLUMN context json; diff --git a/resources/keb/migrations/202410111300_add_created_by_to_binding.down.sql b/resources/keb/migrations/202410111300_add_created_by_to_binding.down.sql new file mode 100644 index 0000000000..a21a811963 --- /dev/null +++ b/resources/keb/migrations/202410111300_add_created_by_to_binding.down.sql @@ -0,0 +1 @@ +ALTER TABLE bindings DROP COLUMN created_by; diff --git a/resources/keb/migrations/202410111300_add_created_by_to_binding.up.sql b/resources/keb/migrations/202410111300_add_created_by_to_binding.up.sql new file mode 100644 index 0000000000..d51c7ba306 --- /dev/null +++ b/resources/keb/migrations/202410111300_add_created_by_to_binding.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE bindings + ADD COLUMN created_by VARCHAR(255); From 34fa7947ef0843f4b234d6421145de554410d000 Mon Sep 17 00:00:00 2001 From: Ksawery Zietara Date: Mon, 14 Oct 2024 10:10:31 +0200 Subject: [PATCH 3/3] Add more test cases for context CreatedBy --- internal/broker/bind_create_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/broker/bind_create_test.go b/internal/broker/bind_create_test.go index 165a8eb965..88a7f8ccf2 100644 --- a/internal/broker/bind_create_test.go +++ b/internal/broker/bind_create_test.go @@ -385,6 +385,7 @@ func TestCreateBindingEndpoint(t *testing.T) { } func TestCreatedBy(t *testing.T) { + emptyStr := "" email := "john.smith@email.com" origin := "origin" tests := []struct { @@ -398,15 +399,30 @@ func TestCreatedBy(t *testing.T) { expected: "", }, { - name: "Only Email is set", + name: "Both Email and Origin are empty", + context: BindingContext{Email: &emptyStr, Origin: &emptyStr}, + expected: "", + }, + { + name: "Origin is nil", context: BindingContext{Email: &email, Origin: nil}, expected: "john.smith@email.com", }, { - name: "Only Origin is set", + name: "Origin is empty", + context: BindingContext{Email: &email, Origin: &emptyStr}, + expected: "john.smith@email.com", + }, + { + name: "Email is nil", context: BindingContext{Email: nil, Origin: &origin}, expected: "origin", }, + { + name: "Email is empty", + context: BindingContext{Email: &emptyStr, Origin: &origin}, + expected: "origin", + }, { name: "Both Email and Origin are set", context: BindingContext{Email: &email, Origin: &origin},