Skip to content

Commit

Permalink
refactor: remove secret type (#703)
Browse files Browse the repository at this point in the history
* refactor: remove secret type

* fix: fix queries for secret

* refactor: add migration to remove type
  • Loading branch information
sbchaos authored Jan 6, 2023
1 parent 8d70686 commit 794803c
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 133 deletions.
2 changes: 1 addition & 1 deletion core/scheduler/service/notification_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (n NotifyService) Push(ctx context.Context, event scheduler.Event) error {
var secret string
switch scheme {
case NotificationSchemeSlack:
secret = secretMap["NOTIFY_SLACK"]
secret = secretMap[tenant.SecretNotifySlack]
case NotificationSchemePagerDuty:
secret = secretMap[strings.ReplaceAll(route, "#", "notify_")]
}
Expand Down
3 changes: 0 additions & 3 deletions core/tenant/dto/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@ package dto

import (
"time"

"github.com/odpf/optimus/core/tenant"
)

type SecretInfo struct {
Name string
Digest string

Type tenant.SecretType
Namespace string

UpdatedAt time.Time
Expand Down
1 change: 0 additions & 1 deletion core/tenant/handler/v1beta1/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ func TestNewSecretsHandler(t *testing.T) {
secretInfo := dto.SecretInfo{
Name: "secret",
Digest: "abcde",
Type: tenant.UserDefinedSecret,
Namespace: ns.String(),
UpdatedAt: time.Date(2022, 9, 22, 0, 0, 0, 0, time.UTC),
}
Expand Down
37 changes: 2 additions & 35 deletions core/tenant/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ const (

SecretStorageKey = "STORAGE"
SecretSchedulerAuth = "SCHEDULER_AUTH"

// SystemDefinedSecret TODO: get rid of system defined secrets
SystemDefinedSecret SecretType = "system"
UserDefinedSecret SecretType = "user"

SecretTypeSystemDefinedPrefix = "_OPTIMUS_"
SecretNotifySlack = "NOTIFY_SLACK"
)

type SecretName string
Expand Down Expand Up @@ -67,29 +62,10 @@ func (p PlainTextSecrets) ToMap() map[string]string {
return secretMap
}

type SecretType string

func SecretTypeFromString(str string) (SecretType, error) {
switch str {
case UserDefinedSecret.String():
return UserDefinedSecret, nil
case SystemDefinedSecret.String():
return SystemDefinedSecret, nil
default:
return "", errors.InvalidArgument(EntitySecret, "unknown type for secret type: "+str)
}
}

func (s SecretType) String() string {
return string(s)
}

type Secret struct {
name SecretName
encodedValue string

_type SecretType

projName ProjectName
namespaceName string
}
Expand All @@ -98,10 +74,6 @@ func (s *Secret) Name() SecretName {
return s.name
}

func (s *Secret) Type() SecretType {
return s._type
}

func (s *Secret) EncodedValue() string {
return s.encodedValue
}
Expand All @@ -114,16 +86,12 @@ func (s *Secret) NamespaceName() string {
return s.namespaceName
}

func NewSecret(name string, _type SecretType, encodedValue string, projName ProjectName, nsName string) (*Secret, error) {
func NewSecret(name string, encodedValue string, projName ProjectName, nsName string) (*Secret, error) {
secretName, err := SecretNameFrom(name)
if err != nil {
return nil, err
}

if _type != UserDefinedSecret && _type != SystemDefinedSecret {
return nil, errors.InvalidArgument(EntitySecret, "invalid secret type")
}

if encodedValue == "" {
return nil, errors.InvalidArgument(EntitySecret, "empty encoded secret")
}
Expand All @@ -135,7 +103,6 @@ func NewSecret(name string, _type SecretType, encodedValue string, projName Proj
return &Secret{
name: secretName,
encodedValue: encodedValue,
_type: _type,
projName: projName,
namespaceName: nsName,
}, nil
Expand Down
32 changes: 4 additions & 28 deletions core/tenant/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,53 +29,29 @@ func TestEntitySecret(t *testing.T) {
})
})
t.Run("Secret", func(t *testing.T) {
t.Run("SecretType", func(t *testing.T) {
t.Run("returns error when unknown type", func(t *testing.T) {
unknown := "unknown"
_, err := tenant.SecretTypeFromString(unknown)
assert.NotNil(t, err)
assert.EqualError(t, err, "invalid argument for entity secret: unknown type for secret type: unknown")
})
t.Run("returns user defined type for valid string", func(t *testing.T) {
typ, err := tenant.SecretTypeFromString(tenant.UserDefinedSecret.String())
assert.Nil(t, err)
assert.Equal(t, tenant.UserDefinedSecret.String(), typ.String())
})
t.Run("returns system defined type for valid string", func(t *testing.T) {
typ, err := tenant.SecretTypeFromString(tenant.SystemDefinedSecret.String())
assert.Nil(t, err)
assert.Equal(t, tenant.SystemDefinedSecret.String(), typ.String())
})
})
t.Run("returns error when name is empty", func(t *testing.T) {
_, err := tenant.NewSecret("", tenant.UserDefinedSecret, "", "", "")
_, err := tenant.NewSecret("", "", "", "")
assert.NotNil(t, err)
assert.EqualError(t, err, "invalid argument for entity secret: secret name is empty")
})
t.Run("returns error when type is invalid", func(t *testing.T) {
_, err := tenant.NewSecret("name", "unknown", "", "", "")
assert.NotNil(t, err)
assert.EqualError(t, err, "invalid argument for entity secret: invalid secret type")
})
t.Run("returns error when encodedValue is empty", func(t *testing.T) {
_, err := tenant.NewSecret("name", tenant.UserDefinedSecret, "", "", "")
_, err := tenant.NewSecret("name", "", "", "")
assert.NotNil(t, err)
assert.EqualError(t, err, "invalid argument for entity secret: empty encoded secret")
})
t.Run("returns error when tenant is invalid", func(t *testing.T) {
_, err := tenant.NewSecret("name", tenant.UserDefinedSecret, "encoded==", "", "")
_, err := tenant.NewSecret("name", "encoded==", "", "")
assert.NotNil(t, err)
assert.EqualError(t, err, "invalid argument for entity secret: invalid tenant details")
})
t.Run("returns secret", func(t *testing.T) {
projName, _ := tenant.ProjectNameFrom("test-project")
nsName := "test-ns"

s, err := tenant.NewSecret("name", tenant.UserDefinedSecret, "encoded==", projName, nsName)
s, err := tenant.NewSecret("name", "encoded==", projName, nsName)

assert.Nil(t, err)
assert.Equal(t, "name", s.Name().String())
assert.Equal(t, "user", s.Type().String())
assert.Equal(t, "encoded==", s.EncodedValue())
assert.Equal(t, projName.String(), s.ProjectName().String())
assert.Equal(t, nsName, s.NamespaceName())
Expand Down
4 changes: 2 additions & 2 deletions core/tenant/service/secret_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s SecretService) Save(ctx context.Context, projName tenant.ProjectName, ns
return errors.InternalError(tenant.EntitySecret, "unable to encrypt the secret", err)
}

item, err := tenant.NewSecret(secret.Name().String(), tenant.UserDefinedSecret, string(encoded), projName, nsName)
item, err := tenant.NewSecret(secret.Name().String(), string(encoded), projName, nsName)
if err != nil {
return err
}
Expand All @@ -54,7 +54,7 @@ func (s SecretService) Update(ctx context.Context, projName tenant.ProjectName,
return errors.InternalError(tenant.EntitySecret, "unable to encrypt the secret", err)
}

item, err := tenant.NewSecret(secret.Name().String(), tenant.UserDefinedSecret, string(encoded), projName, nsName)
item, err := tenant.NewSecret(secret.Name().String(), string(encoded), projName, nsName)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions core/tenant/service/secret_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestSecretService(t *testing.T) {
t.Run("returns the secret in plain text form", func(t *testing.T) {
encodedArr := []byte{63, 158, 156, 88, 23, 217, 166, 22, 135, 126, 204, 156, 107, 103, 217, 229, 58, 37,
182, 124, 36, 80, 59, 94, 141, 238, 154, 6, 197, 70, 227, 117, 185}
sec, err := tenant.NewSecret("name", tenant.UserDefinedSecret, string(encodedArr), projectName, nsName)
sec, err := tenant.NewSecret("name", string(encodedArr), projectName, nsName)
assert.Nil(t, err)

sn, err := tenant.SecretNameFrom("name")
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestSecretService(t *testing.T) {
t.Run("returns the secret in plain text form", func(t *testing.T) {
encodedArr := []byte{63, 158, 156, 88, 23, 217, 166, 22, 135, 126, 204, 156, 107, 103, 217, 229, 58, 37,
182, 124, 36, 80, 59, 94, 141, 238, 154, 6, 197, 70, 227, 117, 185}
sec, _ := tenant.NewSecret("name", tenant.UserDefinedSecret, string(encodedArr), projectName, nsName)
sec, _ := tenant.NewSecret("name", string(encodedArr), projectName, nsName)
secretRepo := new(secretRepo)
secretRepo.On("GetAll", ctx, projectName, nsName).Return([]*tenant.Secret{sec}, nil)
defer secretRepo.AssertExpectations(t)
Expand Down Expand Up @@ -257,7 +257,6 @@ func TestSecretService(t *testing.T) {
secretInfo := dto.SecretInfo{
Name: "name",
Digest: "abcdef",
Type: tenant.UserDefinedSecret,
Namespace: "namespace",
}
secretRepo := new(secretRepo)
Expand Down
1 change: 0 additions & 1 deletion ext/notify/slack/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
)

const (
OAuthTokenSecretName = "NOTIFY_SLACK"
DefaultEventBatchInterval = time.Second * 10
MaxSLAEventsToProcess = 6
)
Expand Down
3 changes: 1 addition & 2 deletions ext/scheduler/airflow/airflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const (
airflowDateFormat = "2006-01-02T15:04:05+00:00"

schedulerHostKey = "SCHEDULER_HOST"
schedulerAuthKey = "SCHEDULER_AUTH"

baseLibFileName = "__lib.py"
jobsDir = "dags"
Expand Down Expand Up @@ -283,7 +282,7 @@ func (s *Scheduler) getSchedulerAuth(ctx context.Context, tnnt tenant.Tenant) (S
return SchedulerAuth{}, err
}

auth, err := s.secretGetter.Get(ctx, tnnt.ProjectName(), tnnt.NamespaceName().String(), schedulerAuthKey)
auth, err := s.secretGetter.Get(ctx, tnnt.ProjectName(), tnnt.NamespaceName().String(), tenant.SecretSchedulerAuth)
if err != nil {
return SchedulerAuth{}, err
}
Expand Down
5 changes: 2 additions & 3 deletions ext/scheduler/airflow/bucket/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@ import (
)

const (
gcsStorageKey = "STORAGE"
scope = "https://www.googleapis.com/auth/cloud-platform"
scope = "https://www.googleapis.com/auth/cloud-platform"
)

func (f *Factory) GetGCSBucket(ctx context.Context, tnnt tenant.Tenant, parsedURL *url.URL) (airflow.Bucket, error) {
spanCtx, span := otel.Tracer("airflow/bucketFactory").Start(ctx, "GetGCSBucket")
defer span.End()

storageSecret, err := f.secretsGetter.Get(spanCtx, tnnt.ProjectName(), tnnt.NamespaceName().String(), gcsStorageKey)
storageSecret, err := f.secretsGetter.Get(spanCtx, tnnt.ProjectName(), tnnt.NamespaceName().String(), tenant.SecretStorageKey)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE secret
ADD COLUMN type VARCHAR(15);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE secret
DROP COLUMN type;
36 changes: 11 additions & 25 deletions internal/store/postgres/tenant/secret_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type SecretRepository struct {
}

const (
secretColumns = `id, name, value, type, project_name, namespace_name, created_at, updated_at`
secretColumns = `id, name, value, project_name, namespace_name, created_at, updated_at`

getAllSecretsInProject = `SELECT ` + secretColumns + `
FROM secret s WHERE project_name = $1`
Expand All @@ -34,8 +34,6 @@ type Secret struct {
Name string
Value string

Type string

ProjectName string
NamespaceName sql.NullString

Expand All @@ -54,7 +52,6 @@ func NewSecret(secret *tenant.Secret) Secret {
return Secret{
Name: secret.Name().String(),
Value: base64cipher,
Type: secret.Type().String(),
ProjectName: secret.ProjectName().String(),
NamespaceName: nsName,
}
Expand All @@ -72,17 +69,12 @@ func (s Secret) ToTenantSecret() (*tenant.Secret, error) {
return nil, err
}

typ, err := tenant.SecretTypeFromString(s.Type)
if err != nil {
return nil, err
}

nsName := ""
if s.NamespaceName.Valid {
nsName = s.NamespaceName.String
}

return tenant.NewSecret(s.Name, typ, string(encrypted), projName, nsName)
return tenant.NewSecret(s.Name, string(encrypted), projName, nsName)
}

func (s Secret) ToSecretInfo() (*dto.SecretInfo, error) {
Expand All @@ -94,11 +86,6 @@ func (s Secret) ToSecretInfo() (*dto.SecretInfo, error) {
digest := cryptopasta.Hash("user defined secrets", encrypted)
base64encoded := base64.StdEncoding.EncodeToString(digest)

typ, err := tenant.SecretTypeFromString(s.Type)
if err != nil {
return nil, err
}

nsName := ""
if s.NamespaceName.Valid {
nsName = s.NamespaceName.String
Expand All @@ -107,7 +94,6 @@ func (s Secret) ToSecretInfo() (*dto.SecretInfo, error) {
return &dto.SecretInfo{
Name: s.Name,
Digest: base64encoded,
Type: typ,
Namespace: nsName,
UpdatedAt: s.UpdatedAt,
}, nil
Expand All @@ -125,9 +111,9 @@ func (s SecretRepository) Save(ctx context.Context, tenantSecret *tenant.Secret)
return errors.Wrap(tenant.EntitySecret, "unable to save secret", err)
}

insertSecret := `INSERT INTO secret (name, value, type, project_name, namespace_name, created_at, updated_at)
VALUES ($1, $2, $3, $4, $5, NOW(), NOW())`
_, err = s.db.Exec(ctx, insertSecret, secret.Name, secret.Value, secret.Type, secret.ProjectName, secret.NamespaceName)
insertSecret := `INSERT INTO secret (name, value, project_name, namespace_name, created_at, updated_at)
VALUES ($1, $2, $3, $4, NOW(), NOW())`
_, err = s.db.Exec(ctx, insertSecret, secret.Name, secret.Value, secret.ProjectName, secret.NamespaceName)

if err != nil {
return errors.Wrap(tenant.EntitySecret, "unable to save secret", err)
Expand All @@ -147,10 +133,10 @@ func (s SecretRepository) Update(ctx context.Context, tenantSecret *tenant.Secre
return errors.Wrap(tenant.EntitySecret, "unable to update secret", err)
}

updateSecret := `UPDATE secret SET value=$1, type=$2, updated_at=NOW()
WHERE project_name = $3 AND name=$4`
updateSecret := `UPDATE secret SET value=$1, updated_at=NOW()
WHERE project_name = $2 AND name=$3`

_, err = s.db.Exec(ctx, updateSecret, secret.Value, secret.Type, secret.ProjectName, secret.Name)
_, err = s.db.Exec(ctx, updateSecret, secret.Value, secret.ProjectName, secret.Name)
if err != nil {
return errors.Wrap(tenant.EntitySecret, "unable to update secret", err)
}
Expand All @@ -167,7 +153,7 @@ AND project_name = $2
AND (namespace_name IS NULL OR namespace_name = $3)`

err := s.db.QueryRow(ctx, getSecretByNameQuery, name, projName, nsName).
Scan(&secret.ID, &secret.Name, &secret.Value, &secret.Type,
Scan(&secret.ID, &secret.Name, &secret.Value,
&secret.ProjectName, &secret.NamespaceName, &secret.CreatedAt, &secret.UpdatedAt)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
Expand Down Expand Up @@ -207,7 +193,7 @@ WHERE project_name = $1 AND (namespace_name IS NULL or namespace_name = $2)`
var tenantSecrets []*tenant.Secret
for rows.Next() {
var sec Secret
err := rows.Scan(&sec.ID, &sec.Name, &sec.Value, &sec.Type,
err := rows.Scan(&sec.ID, &sec.Name, &sec.Value,
&sec.ProjectName, &sec.NamespaceName, &sec.CreatedAt, &sec.UpdatedAt)
if err != nil {
return nil, errors.Wrap(tenant.EntitySecret, "error in GetAll", err)
Expand Down Expand Up @@ -258,7 +244,7 @@ func (s SecretRepository) GetSecretsInfo(ctx context.Context, projName tenant.Pr
var secretInfo []*dto.SecretInfo
for rows.Next() {
var sec Secret
err := rows.Scan(&sec.ID, &sec.Name, &sec.Value, &sec.Type,
err := rows.Scan(&sec.ID, &sec.Name, &sec.Value,
&sec.ProjectName, &sec.NamespaceName, &sec.CreatedAt, &sec.UpdatedAt)
if err != nil {
return nil, errors.Wrap(tenant.EntitySecret, "error in GetAll", err)
Expand Down
Loading

0 comments on commit 794803c

Please sign in to comment.