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

chart: default email value for s3gw-letsencrypt-issuer clusterIssuer is forbidden by LE #596

Closed
thehejik opened this issue Jun 23, 2023 · 2 comments · Fixed by s3gw-tech/s3gw-charts#107
Assignees
Labels
area/kubernetes k8s and related kind/bug Something isn't working kind/quality Quality improvements, Refactoring, Automation via CI, E2E, Integration, CLI or REST API release-note Should be explicitly mentioned in the release notes triage/next-candidate This could be moved to the next milestone
Milestone

Comments

@thehejik
Copy link

Describe the bug
When s3gw is installed with default values over helm (without requesting LE certificate for its domain) the clusterIssuer *-s3gw-letsencrypt-issuer is in Unavailable state.

To Reproduce Steps to reproduce the behavior:
Install with default values:

helm upgrade --install s3gw s3gw/s3gw --namespace s3gw \
    --create-namespace --set ingress.enabled=true \
    --set publicDomain=domain --set ui.publicDomain=domain

Check output of:

$ kubectl get clusterIssuer -lapp.kubernetes.io/name=s3gw        
NAME                           READY   AGE
s3gw-s3gw-self-signed-issuer   True    2m38s
s3gw-s3gw-letsencrypt-issuer   False   2m38s
s3gw-s3gw-issuer               True    2m38s

See details of the Failed clusterIssuer:

$ kubectl get clusterIssuer s3gw-s3gw-letsencrypt-issuer -o yaml
...
spec:
  acme:
    email: mail@example.com
 ...
status:
  acme: {}
  conditions:
  - lastTransitionTime: "2023-06-23T07:38:51Z"
    message: 'Failed to register ACME account: 400 urn:ietf:params:acme:error:invalidContact:
      Error creating new account :: invalid contact domain. Contact emails @example.com
      are forbidden'
    observedGeneration: 1
    reason: ErrRegisterACMEAccount
    status: "False"
    type: Ready

Expected behavior
All the resources should be in Ready state.
Maybe a different email than mail@example.com should be used here https://github.com/aquarist-labs/s3gw-charts/blob/main/charts/s3gw/values.yaml#L17

@giubacc
Copy link

giubacc commented Jun 23, 2023

@m-ildefons I don't remember if we explicitly decided to put an example.com email to make the failure explicit and force the user to put a value here.
We can decide to change this if we think we should.

@m-ildefons
Copy link
Contributor

I think I put it there, expecting anyone who is actually using it to recognize it as placeholder and change it. Clearly that was wrong and we should build a function similar to the ones used for (ui.)publicDomain that fails the templating if it detects that the user didn't input an acceptable value.
I still believe the default should remain at example.com, since it's impossible to give a default value that would be accepted by letsencrypt.

@m-ildefons m-ildefons self-assigned this Jun 23, 2023
@m-ildefons m-ildefons added kind/bug Something isn't working release-note Should be explicitly mentioned in the release notes triage/next-candidate This could be moved to the next milestone kind/quality Quality improvements, Refactoring, Automation via CI, E2E, Integration, CLI or REST API area/kubernetes k8s and related and removed triage/waiting Waiting for triage labels Jun 23, 2023
@m-ildefons m-ildefons added this to the v0.18.0 milestone Jun 23, 2023
m-ildefons referenced this issue in m-ildefons/s3gw-charts Jun 23, 2023
When using the Letsencrypt TLS issuer, a valid email address (the one of
the account requesting the certificate) is required in the .email field
of the values.yaml.
This change introduces a templating check that fails the installation of
the s3gw if the letsencrypt issuer is to be used with the default
(invalid) email, reminding the user to set a valid one.
It also avoids deploying both the self-signed and letsencrypt issuers at
the same time. In case the self-signed issuer is desired it unnecessary
to deploy the letsencrypt issuer as well and with no valid email given
(which isn't necessary for the self signed issuer), the letsencrypt
issued would not deploy successfully.

Fixes: aquarist-labs/s3gw#596
Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes k8s and related kind/bug Something isn't working kind/quality Quality improvements, Refactoring, Automation via CI, E2E, Integration, CLI or REST API release-note Should be explicitly mentioned in the release notes triage/next-candidate This could be moved to the next milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants