Replace omg.howdoi.website detection and use provided issuer instead #590

manno · 2021-07-07T16:02:05Z

add arg --tls-issuer to specify the name of the cert manager issuer to use
add args to CLI and server to toggle the creation and use of the node
port workaround for the internal registry
(--enable-internal-registry-node-port,
--use-internal-registry-node-port)
remove decision on tekton trust, always add certificate to tekton and
add TODO to remove workaround once we can use the mesh
epinio install will use current TRACE_LEVEL for server

No tests were added since we don't have tests for 'production'/letsencrypt installations. Really, we have no tests for installation at all.

fixes #590

manno · 2021-07-09T16:34:19Z

no longer green after rebase on main

internal/application/application.go

jimmykarily · 2021-07-12T10:51:34Z

internal/cli/server.go

+	viper.BindPFlag("tls-issuer", flags.Lookup("tls-issuer"))
+	viper.BindEnv("tls-issuer", "TLS_ISSUER")
+
+	flags.Bool("use-internal-registry-node-port", true, "(USE_INTERNAL_REGISTRY_NODE_PORT) Use the internal registry via a node port")


There is no point in setting this to true if enable-internal-registry-node-port was set to false at installation time right?
It seems to me, the only reason we need this flag is because we have no place to store the value of the enable-internal-registry-node-port flag that was used. Is there a way to detect what value was used? For example, check the registry service to see what type it is?

How does the user define this for the server? Epinio sets up the server command automatically inside the server deployment, the user doesn't have a way to provide flags. The install command is the one that creates the server command and that one has access to the --enable-internal-registry-node-port flag. Maybe we should use that to define the --use-internal-registry-node-port?

Yes, that was the plan. Let me check if I forgot to do this...

Ah here it is, some awful indirection, but all variable names match the pattern (snake case, kebap case, ...)

https://github.com/epinio/epinio/pull/590/files#diff-892d335f47fd05c8157a81b96c028b3d6c1a88492ca015135d24fe86a1ede046R308-R309

So the client puts this as an env var into the epinio deployment yaml via our ##substitution##. (I feel we should invest in real templates soon;)

I thought 'use' was more approriate for the server flag, but maybe it should be the same name as the client flag 'enable' ?

maybe use should be used in all cases? I mean, they use it in a different way but they both use it somehow. Otoh, I'm still thinking we could get rid of the server flag but inspecting the registry service. It's only used in one place (the staging api) in order to decide what the registry URL should look like.

jimmykarily · 2021-07-12T11:26:30Z

I verified that I can use another domain and still use self-signed certificates with:

/dist/epinio-linux-amd64 install --skip-default-org --verbosity=1 --system-domain 10.86.4.38.nip.io

jimmykarily · 2021-07-12T11:52:39Z

Since there is no auto-detection of "production" setup anymore, we need to document the recommended way to deploy in production. That should prominent in the README.md (even if it's just a link to another page).

jimmykarily · 2021-07-12T13:35:35Z

I face this issue:

I create a gke cluster and get a kubeconfig to talk to it
I run this command to setup ingress: ./dist/epinio-linux-amd64 install-ingress
I run this command to install Epinio:

TRACE_LEVEL=100 ./dist/epinio-linux-amd64 install --verbosity=1  --enable-internal-registry-node-port=false --tls-issuer letsencrypt-production  --system-domain pr-590.mydomain.org

I get this error:


🕞  Starting app.kubernetes.io/name=epinio-server in epinio .

🕞    Starting pod epinio-server-5cb687c87f-ktqw7 in epinio ...........................
✔️  Registry deployed
2021/07/12 16:32:13 InstallClient/Install "level"=2 "msg"="return"  
...

✔️  Gitea deployed
2021/07/12 16:32:14 InstallClient/Install "level"=2 "msg"="return"  
....................................................................................................................................................

🕞   Applying tekton staging resources .
2021/07/12 16:33:28 InstallClient/Install "level"=2 "msg"="return"  
panic: Deployment tekton failed with error: Applying tekton staging resources failed:
: Failed to get registry CA from tekton-staging namespace: failed to decode certificate
failed find PEM data


goroutine 29 [running]:
github.com/epinio/epinio/internal/cli/clients.(*InstallClient).Install.func1(0xc0007ef620, 0x21012e0, 0xc0000c0000, 0x2117f40, 0xc000276820, 0x21235e0, 0xc0000d4400, 0xc000040e74)
	/home/dimitris/workspace/suse/epinio/internal/cli/clients/install_client.go:148 +0x263
created by github.com/epinio/epinio/internal/cli/clients.(*InstallClient).Install
	/home/dimitris/workspace/suse/epinio/internal/cli/clients/install_client.go:145 +0xe1b

removes the unused cmd.Flags from NewInstallClient

The certificates CRD is deleted via the owner ref

* add arg `--tls-issuer` to specify the name of the cert manager issuer to use * add args to CLI and server to toggle the creation and use of the node port workaround for the internal registry (`--enable-internal-registry-node-port`, `--use-internal-registry-node-port`) * remove decision on tekton trust, always add certificate to tekton and add TODO to remove workaround once we can use the mesh * epinio install will use current TRACE_LEVEL for server

With automatic ownership we get clean up for free

Previously didn't work, because go-template returned not an empty string, but "<no value>" Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

ACME generated certs from cert-manager don't have a ca.crt key. * tektoncd/catalog#757 Co-authored-by: Mario Manno <mario.manno@suse.com>

Was not used anymore Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

Use a fixed string for users home dir Co-authored-by: Mario Manno <mario.manno@suse.com>

…ethod and retry on one more case (reverting a previous commit which removed it) Signed-off-by: Dimitris Karakasilis <DKarakasilis@suse.com> Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

manno force-pushed the 564-custom-issuer branch 3 times, most recently from 19bf081 to 4dd6734 Compare July 9, 2021 10:13

manno marked this pull request as ready for review July 9, 2021 11:08

manno force-pushed the 564-custom-issuer branch from 4dd6734 to 41bba3f Compare July 9, 2021 12:10

manno changed the title ~~564 custom issuer~~ Replace omg.howdoi.website detection and use provided issuer instead Jul 9, 2021

manno force-pushed the 564-custom-issuer branch 2 times, most recently from f9d5d5f to 2ccc1d1 Compare July 9, 2021 16:12

jimmykarily reviewed Jul 12, 2021

View reviewed changes

internal/application/application.go Outdated Show resolved Hide resolved

jimmykarily reviewed Jul 12, 2021

View reviewed changes

manno force-pushed the 564-custom-issuer branch from 2ccc1d1 to 72e9c25 Compare July 12, 2021 15:45

jimmykarily mentioned this pull request Jul 13, 2021

Use a CA issuer instead of self-signed certificates #564

Closed

Mario Manno added 10 commits July 13, 2021 10:48

Move LocalRegistry out of Gitea package

0ca59b9

Pass cmd.Flags instead of cmd to commands

0ad48be

removes the unused cmd.Flags from NewInstallClient

Remove unused code from option

551bab9

Add getters for global, needed options

c8785fa

Add createNodePort value to internal registry's helm chart

6e75291

Move workload's cert deletion into server

2126796

The certificates CRD is deleted via the owner ref

ConfigUpdate always stores the current CA

2db5c91

Cert-manager certificates own cert secrets

f290758

With automatic ownership we get clean up for free

Try to fix flaky test

a216cc8

manno force-pushed the 564-custom-issuer branch from 72e9c25 to 3eee654 Compare July 13, 2021 10:40

Mario Manno and others added 4 commits July 13, 2021 14:02

Waiting for epinio-registry-tls to have data

024d0c5

Previously didn't work, because go-template returned not an empty string, but "<no value>" Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

Only mount untrusted CA in tekton

ab814b7

ACME generated certs from cert-manager don't have a ca.crt key. * tektoncd/catalog#757 Co-authored-by: Mario Manno <mario.manno@suse.com>

Remove Quarks

7c33bef

Was not used anymore Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

Update cli reference docs

8fcac55

Use a fixed string for users home dir Co-authored-by: Mario Manno <mario.manno@suse.com>

manno force-pushed the 564-custom-issuer branch from 3eee654 to 8fcac55 Compare July 13, 2021 12:22

Handle installation error in tests, introduce wait for cert manager m…

3d48721

…ethod and retry on one more case (reverting a previous commit which removed it) Signed-off-by: Dimitris Karakasilis <DKarakasilis@suse.com> Co-authored-by: Dimitris Karakasilis <DKarakasilis@suse.com>

manno force-pushed the 564-custom-issuer branch from 4970d86 to 3d48721 Compare July 13, 2021 14:26

andreas-kupries mentioned this pull request Jul 14, 2021

Create private epinio CA #613

Merged

jimmykarily approved these changes Jul 14, 2021

View reviewed changes

manno merged commit e350a06 into main Jul 14, 2021

manno deleted the 564-custom-issuer branch July 14, 2021 08:06

andreas-kupries mentioned this pull request Jul 14, 2021

Update docs for tls-issuer arg #614

Merged

jimmykarily mentioned this pull request Jul 20, 2021

Replace omg.howdoi.website with an epinio.io subdomain #271

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace omg.howdoi.website detection and use provided issuer instead #590

Replace omg.howdoi.website detection and use provided issuer instead #590

manno commented Jul 7, 2021 •

edited

Loading

manno commented Jul 9, 2021

jimmykarily Jul 12, 2021 •

edited

Loading

jimmykarily Jul 12, 2021

manno Jul 12, 2021

manno Jul 12, 2021

jimmykarily Jul 13, 2021

jimmykarily commented Jul 12, 2021

jimmykarily commented Jul 12, 2021

jimmykarily commented Jul 12, 2021 •

edited

Loading

Replace omg.howdoi.website detection and use provided issuer instead #590

Replace omg.howdoi.website detection and use provided issuer instead #590

Conversation

manno commented Jul 7, 2021 • edited Loading

manno commented Jul 9, 2021

jimmykarily Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

jimmykarily Jul 12, 2021

Choose a reason for hiding this comment

manno Jul 12, 2021

Choose a reason for hiding this comment

manno Jul 12, 2021

Choose a reason for hiding this comment

jimmykarily Jul 13, 2021

Choose a reason for hiding this comment

jimmykarily commented Jul 12, 2021

jimmykarily commented Jul 12, 2021

jimmykarily commented Jul 12, 2021 • edited Loading

manno commented Jul 7, 2021 •

edited

Loading

jimmykarily Jul 12, 2021 •

edited

Loading

jimmykarily commented Jul 12, 2021 •

edited

Loading