Skip to content

Create secret #97

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.

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 10 commits into from
Feb 17, 2021
Merged

Create secret #97

merged 10 commits into from
Feb 17, 2021

Conversation

LimKianAn
Copy link
Contributor

@LimKianAn LimKianAn commented Feb 15, 2021

for issue #93

@LimKianAn LimKianAn changed the title WIP: Complete feature (not tested) WIP: Create secret Feb 15, 2021
const SecretNamespace = "pgaas"

func (p *Postgres) ToSecret(src *corev1.SecretList) (*corev1.Secret, error) {
new := &corev1.Secret{}
Copy link
Contributor

Choose a reason for hiding this comment

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

new is a reserved word and irritates reading

return p.ToPeripheralResourceName() + "-passwords"
}

func (p *Postgres) ToSecretNamesapcedName() *types.NamespacedName {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

}
}

func (p *Postgres) ToSecretName() string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToUserPasswordsSecretName()

@@ -162,6 +162,12 @@ func (r *PostgresReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
return ctrl.Result{}, fmt.Errorf("unable to create or update corresponding CRD ClusterwideNetworkPolicy: %W", err)
}

if instance.Spec.SecretName == "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the check. Always update the secret, replacing the Data block.

@LimKianAn
Copy link
Contributor Author

LimKianAn commented Feb 16, 2021

Decision: Secret in control-plane cluster is in the same namespace as Postgres. For example: pgaas

Suggestion:

  1. The name of Secret should be the name of Postgres plus a suffix.
  2. secretName should not be in the spec. of Postgres. It's a direct derivation of unknown spec. of Postgres.

@LimKianAn LimKianAn requested a review from majst01 February 16, 2021 19:39
@LimKianAn LimKianAn changed the title WIP: Create secret Create secret Feb 16, 2021
}
log.Info("secret created or updated", "operation result", result)

if result == controllerutil.OperationResultCreated {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right here as well: The name will probably not change and could be derived automativally (by convention). Hence, we would not need the field secretName in the spec.

What we should do, however, is export the String-Suffix we use to generate the secret name (currently -password) so it the same string will be used in the cloudctl/cloud-api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@majst01
We added an exported func ToUserPasswordsSecretName to Postgres. Can we remove secretName from the spec.?

return ctrl.Result{}, nil
}

// SetupWithManager registers this controller for reconciliation of Postgresql resources
func (r *StatusReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&zalando.Postgresql{}).
// Owns(&corev1.Secret{}). // todo: better the watching on secrets that weren't generated by this controller
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not used, please remove.

@LimKianAn LimKianAn requested a review from eberlep February 17, 2021 12:21
@eberlep eberlep merged commit 3abb733 into main Feb 17, 2021
@eberlep eberlep deleted the create-secret branch February 17, 2021 12:22
@eberlep eberlep linked an issue Feb 17, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide Credentials for Created Databases
3 participants