Skip to content

Delete secrets created if it fails on tenant creation #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 10, 2020
Merged

Delete secrets created if it fails on tenant creation #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,13 @@ github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQ
github.com/djherbis/atime v1.0.0 h1:ySLvBAM0EvOGaX7TI4dAM5lWj+RdJUCKtGSEHN8SGBg=
github.com/djherbis/atime v1.0.0/go.mod h1:5W+KBIuTwVGcqjIfaTwt+KSYX1o6uep8dtevevQP/f8=
github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E=
github.com/docker/cli v0.0.0-20191017083524-a8ff7f821017 h1:2HQmlpI3yI9deH18Q6xiSOIjXD4sLI55Y/gfpa8/558=
github.com/docker/cli v0.0.0-20191017083524-a8ff7f821017/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8=
github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
github.com/docker/docker v0.7.3-0.20190327010347-be7ac8be2ae0/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/docker v1.4.2-0.20190924003213-a8608b5b67c7 h1:Cvj7S8I4Xpx78KAl6TwTmMHuHlZ/0SM60NUneGJQ7IE=
github.com/docker/docker v1.4.2-0.20190924003213-a8608b5b67c7/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/docker-credential-helpers v0.6.3 h1:zI2p9+1NQYdnG6sMU26EX4aVGlqbInSQxQXLvzJ4RPQ=
github.com/docker/docker-credential-helpers v0.6.3/go.mod h1:WRaJzqw3CTB9bk10avuGsjVBZsD05qeibJ1/TYlvc0Y=
github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec=
github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
Expand Down Expand Up @@ -491,6 +494,7 @@ github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.4.1 h1:/exdXoGamhu5ONeUJH0deniYLWYvQwW66yvlfiiKTu0=
github.com/google/go-cmp v0.4.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-containerregistry v0.1.2 h1:YjFNKqxzWUVZND8d4ItF9wuYlE75WQfECE7yKX/Nu3o=
github.com/google/go-containerregistry v0.1.2/go.mod h1:GPivBPgdAyd2SU+vf6EpsgOtWDuPqjW0hJZt4rNdTZ4=
github.com/google/go-github/v28 v28.1.1/go.mod h1:bsqJWQX05omyWVmc00nEUql9mhQyv38lDZ8kPZcQVoM=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
Expand Down Expand Up @@ -622,6 +626,7 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:
github.com/imdario/mergo v0.3.5 h1:JboBksRwiiAJWvIYJVo46AfV+IAIKZpfrSzVKj42R4Q=
github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg=
github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf h1:WfD7VjIE6z8dIvMsI4/s+1qr5EL+zoIGev1BQj1eoJ8=
github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf/go.mod h1:hyb9oH7vZsitZCiBt0ZvifOrB+qc8PS5IiilCIb87rg=
Expand Down Expand Up @@ -1252,6 +1257,7 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand Down
25 changes: 19 additions & 6 deletions restapi/admin_tenants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ func getListTenantsResponse(session *models.Principal, params admin_api.ListTena
return listT, nil
}

func getTenantCreatedResponse(session *models.Principal, params admin_api.CreateTenantParams) (*models.CreateTenantResponse, *models.Error) {
func getTenantCreatedResponse(session *models.Principal, params admin_api.CreateTenantParams) (response *models.CreateTenantResponse, mError *models.Error) {
tenantReq := params.Body
minioImage := tenantReq.Image
ctx := context.Background()
Expand Down Expand Up @@ -478,11 +478,24 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create
if err != nil {
return nil, prepareError(err)
}
// delete secrets created if an error occurred during tenant creation,
defer func() {
if mError != nil {
log.Printf("deleting secrets created for failed tenant: %s if any\n", tenantName)
opts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", operator.TenantLabel, tenantName),
}
err = clientSet.CoreV1().Secrets(ns).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
if err != nil {
log.Println("error deleting tenant's secrets:", err)
}
}
}()

var envrionmentVariables []corev1.EnvVar
// Check the Erasure Coding Parity for validity and pass it to Tenant
if tenantReq.ErasureCodingParity > 0 {
if tenantReq.ErasureCodingParity < 2 && tenantReq.ErasureCodingParity > 8 {
if tenantReq.ErasureCodingParity < 2 || tenantReq.ErasureCodingParity > 8 {
return nil, prepareError(errorInvalidErasureCodingValue)
}
envrionmentVariables = append(envrionmentVariables, corev1.EnvVar{
Expand Down Expand Up @@ -713,7 +726,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create

if tenantReq.ImagePullSecret != "" {
imagePullSecret = tenantReq.ImagePullSecret
} else if imagePullSecret, err = setImageRegistry(ctx, tenantName, tenantReq.ImageRegistry, clientSet.CoreV1(), ns); err != nil {
} else if imagePullSecret, err = setImageRegistry(ctx, tenantReq.ImageRegistry, clientSet.CoreV1(), ns, tenantName); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how was this working before? o_O

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nv,

return nil, prepareError(err)
}
// pass the image pull secret to the Tenant
Expand Down Expand Up @@ -752,7 +765,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create
return nil, prepareError(err)
}
}
response := &models.CreateTenantResponse{
response = &models.CreateTenantResponse{
AccessKey: accessKey,
SecretKey: secretKey,
}
Expand All @@ -768,7 +781,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create

// setImageRegistry creates a secret to store the private registry credentials, if one exist it updates the existing one
// returns the name of the secret created/updated
func setImageRegistry(ctx context.Context, tenantName string, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace string) (string, error) {
func setImageRegistry(ctx context.Context, req *models.ImageRegistry, clientset v1.CoreV1Interface, namespace, tenantName string) (string, error) {
if req == nil || req.Registry == nil || req.Username == nil || req.Password == nil {
return "", nil
}
Expand Down Expand Up @@ -839,7 +852,7 @@ func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, cli
minInst.Spec.ImagePullSecret.Name = params.Body.ImagePullSecret
} else {
// update the image pull secret content
if _, err := setImageRegistry(ctx, params.Tenant, imageRegistryReq, clientset, namespace); err != nil {
if _, err := setImageRegistry(ctx, imageRegistryReq, clientset, namespace, params.Tenant); err != nil {
log.Println("error setting image registry secret:", err)
return err
}
Expand Down
27 changes: 1 addition & 26 deletions restapi/admin_tenants_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,6 @@ func getTenantUpdateEncryptionResponse(session *models.Principal, params admin_a
// getKESConfiguration will generate the KES server certificate secrets, the tenant client secrets for mTLS authentication between MinIO and KES and the
// kes-configuration.yaml file used by the KES service (how to connect to the external KMS, eg: Vault, AWS, Gemalto, etc)
func getKESConfiguration(ctx context.Context, clientSet K8sClientI, ns string, encryptionCfg *models.EncryptionConfiguration, secretName, tenantName string, autoCert bool) (kesConfiguration *operator.KESConfig, err error) {
// Secrets used by the MiniO tenant service
//
// tenantExternalClientCertSecretName is the name of the secret that will store the certificates for mTLS between MinIO and the KES, eg: app.key and app.crt
tenantExternalClientCertSecretName := fmt.Sprintf("%s-tenant-external-client-cert", secretName)
// Secrets used by the KES service
//
// kesExternalCertSecretName is the name of the secret that will store the certificates for TLS in the KES server, eg: server.key and server.crt
Expand All @@ -195,28 +191,7 @@ func getKESConfiguration(ctx context.Context, clientSet K8sClientI, ns string, e
kesClientCertSecretName := fmt.Sprintf("%s-kes-client-cert", secretName)
// kesConfigurationSecretName is the name of the secret that will store the configuration file, eg: kes-configuration.yaml
kesConfigurationSecretName := fmt.Sprintf("%s-kes-configuration", secretName)
// if there's an error during this process we delete all KES configuration secrets
defer func() {
if err != nil {
errDelete := clientSet.deleteSecret(ctx, ns, tenantExternalClientCertSecretName, metav1.DeleteOptions{})
if errDelete != nil {
log.Print(errDelete)
}
errDelete = clientSet.deleteSecret(ctx, ns, kesExternalCertSecretName, metav1.DeleteOptions{})
if errDelete != nil {
log.Print(errDelete)
}
errDelete = clientSet.deleteSecret(ctx, ns, kesClientCertSecretName, metav1.DeleteOptions{})
if errDelete != nil {
log.Print(errDelete)
}
errDelete = clientSet.deleteSecret(ctx, ns, kesConfigurationSecretName, metav1.DeleteOptions{})
if errDelete != nil {
log.Print(errDelete)
}
return
Comment on lines -201 to -217
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did this code got moved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cause that will be handled by the defer function on create Tenant func, I also saw that this function was only being used there. Do you think this will be used in other funcs?

}
}()

kesConfiguration = &operator.KESConfig{
Image: "minio/kes:v0.11.0",
Replicas: 1,
Expand Down