Skip to content
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

Formatter #51

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Formatter #51

wants to merge 5 commits into from

Conversation

poelzi
Copy link

@poelzi poelzi commented Aug 3, 2021

This PR adds the possibility to format secret values according to rules defined
in the new format.secret-generator.v1.mittwald.de/ annotation prefix.
This allows secret-generator to generate specially formatted values containing the
newly generated secret.

Some charts or software require the username and password to be formatting in
form of a uri. With this addition, it is now possible to use the secret generator with
nearly every chart out there.

@mittwald-machine
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

@poelzi
Copy link
Author

poelzi commented Sep 7, 2021

@martin-helmich can I have a review please ? :)

With this addition, it is possible to the password generator for more helm charts that require special formatted secrets like url formatted secrets. I think this is a very nice addition

@martin-helmich
Copy link
Member

Hey Daniel,

thanks for your contribution 👍 and my apologies for the delay! 🙄🙏

TBH, I was already having mixed feelings before concerning the multitude of different annotations. Configuring the secret generation via annotation is already tedious and only provides limited extensibility.

In #35, we've added the possibility to define generated secrets as Custom Resources, which gives us way more possibilities to define their exact properties in a structured and well-defined way.

I'm thinking if instead, we could possibly extend the CR definition to accept templated expressions (maybe feature-freezing the generation by annotation entirely, in favour of the CR-based generation):

apiVersion: secretgenerator.mittwald.de/v1alpha1
kind: StringSecret
metadata:
  name: example-pw
spec:
  forceRegenerate: false
  data:
    user: testuser
  fields:
    - fieldName: password
      encoding: base64
      length: 32
  dataTemplates:
    - fieldName: loginUri
      template: >
        http://{{ .data.user | base64decode | urlquery }}:{{.data.password | base64decode | urlquery}}@localhost/

Any thoughts are welcome. 🙂

@mittwald-machine
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

This flake file allowes you to enter a working dev environment
with "nix shell". It includes the very old operator-sdk, helm, kind, etc
Allows the user to specify a template which is rendered with go
template engine. The generated values are added to the Secret.

This allows secretgenerator to create passwords in special syntax
like url's
@poelzi
Copy link
Author

poelzi commented Oct 15, 2021

@martin-helmich I rewrote the patch to use the new CRD. I think the new solution is quite elegant and allows even one template to use the output of a previous template.

@poelzi poelzi force-pushed the formatter branch 4 times, most recently from ae687ff to 1ce3af2 Compare October 15, 2021 14:31
Either codeclimate complains about the number of arguments or similarity in
code. Go error handling is a mess and therefore lots of ducplated code.
If codeclimate would distingish between internal and external api, things
would be easier
@poelzi
Copy link
Author

poelzi commented Oct 15, 2021

:( this codeclimate is the most annoying code checker i have seen so far. so creating a config file and adjusting one test somehow changes the complete behavior ?
I tried different refactoring of the code, I either get complains about code duplication or about 5 arguments even on a internal function, is somehow to much.
I tried to increase argument limit, but this does not work eigther

@hensur
Copy link
Member

hensur commented Oct 18, 2021

@poelzi Don't worry about code climate. I think I'm going to remove it from this repo anyways. As you say, it is quite annoying and not that useful for us.

I'm sorry that you had to deal with it.

@mittwald-machine
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

@mittwald-machine
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants