Extract unrelated code fixes/features out of k8s native apply work #196

jeff-mccoy · 2021-12-09T10:06:21Z

Breaking Changes:

localhost is no longer a valid option for cluster ingress when initializing a zarf cluster. Instead you have to use a 127.0.0.1 or some other local ip found via ifconfig

Fixes:

No longer depends on 127.0.0.1 local bindings for the registry / gitops service
- should fix Bug: (or docs issue): Games Example doesn't work on zarf 0.13.3 when using cert imported from Lets Encrypt Free CA #193
Resolve outstanding issues with image hostname swapping and

Features:

Adds before and after script options when defining a zarf.yaml with an optional retry flag
Add symlink to ZarfFile for creating links to places files
Add template boolean to ZarfFile to allow injection of zarf variables into text files
Adds a new zarf tool command to print out config schema and commit the output to the repo (will need to make a git hook or something later on)
Changes zarf destroy command to run any script that starts with zarf-clean instead of only running the k3s-remove script
Add new ZarfState and .zarf-state.yaml for persisting host information from zarf init to zarf package deploy
Remove all hard-coded logic for k3s install, now uses only standard zarf component features like everything else

Misc:

Upgrades k3s from v1.21.2 to v1.21.6
Adds optional regex filter for when performing RecursiveFileList()
Adds more description to the components in zarf.yaml
Renames type ZarfConfig to ZarfPackage in the config pkg
Handful of general code organizing changes (moving yaml related functions to the ...../utils/yaml.go, etc.)
Expose execCommand() with stdout control
Move traefik to standalone component and drop the internal k3s install of traefik
Use the airgap tarball of K3s instead of manually listing images

cli/internal/images/push.go

RothAndrew · 2021-12-09T21:32:04Z

I think we do need to add in blocking the user from using localhost as the host. When I try it I get this error:

zarf tools registry catalog localhost gives me this:

If we do need to block the user from using localhost, we should also add a "tooltip" or some other kind of help text. Something like:

Enter a host DNS entry or IP Address for the cluster ingress.
Note: Due to <InsertReasonHere> you can't use "localhost" or "127.0.0.1".
Here are some suggestions that I've determined might work for you: [10.0.2.15]

YrrepNoj · 2021-12-09T21:38:54Z

I think we do need to add in blocking the user from using localhost as the host. When I try it I get this error:

zarf tools registry catalog localhost gives me this:

If we do need to block the user from using localhost, we should also add a "tooltip" or some other kind of help text. Something like:
Enter a host DNS entry or IP Address for the cluster ingress.
Note: Due to <InsertReasonHere> you can't use "localhost" or "127.0.0.1".
Here are some suggestions that I've determined might work for you: [10.0.2.15]

There's a chance you might have been using a different zarf version. I believe this PR prevents you from using localhost as the ingress.

EDIT: Nope.. I didn't realize Jeff pushed a new commit out a few hours ago specifically allowing localhost again. I agree with Andy, maybe should probably consider just disallowing it.

jeff-mccoy · 2021-12-09T22:01:10Z

So on the localhost thing, nothing should be changing behavior now that would break that. But it's very important we understand you can't use localhosts and 127.0.0.1 interchangeably due to the certs not auto-magically adding those SAN values. Can you show me your .zarf-state.yaml when you're having this issue? Also I do think there might be some issues using localhost vs 127.0.0.1 that we've encountered before--we probably should block the actual DNS localhost. Thoughts?

RothAndrew · 2021-12-09T22:13:50Z

@jeff-mccoy You're right that 127.0.0.1 works, but localhost doesn't.

When I use --host localhost here's my .zarf-state.yaml, with the errors already mentioned above

kind: ZarfState
tls:
  certPublicPath: zarf-pki/zarf-server.crt
  certPrivatePath: zarf-pki/zarf-server.key
  host: localhost

jeff-mccoy · 2021-12-09T22:19:12Z

Yeah this was an issue we ran into before if I recall, why we switched everything to hard-coded 127.0.0.1 vs localhost. How do you feel about just blocking localhost for now so we don't totally change how you use this version of zarf?

RothAndrew · 2021-12-09T22:20:47Z

I'm okay with allowing 127.0.0.1 and blocking localhost. Let's do that.

Question: Once the rest of the native apply stuff is in, are we going to have to block 127.0.0.1 also?

jeff-mccoy · 2021-12-09T22:22:40Z

as of today yes, there may be some clever workaround, but I haven't found it yet. 😢

jeff-mccoy · 2021-12-09T22:38:12Z

@RothAndrew made that change in the last commit

cli/cmd/initialize.go

RothAndrew · 2021-12-09T22:59:49Z

For some reason, in the Big Bang example, the flux_generated.yaml manifest is not being deployed. I can see it there in the /var/lib/rancher/k3s/server/manifests folder, but it doesn't deploy.

Deploying the file with kubectl apply -f flux_generated.yaml works fine, and that unblocks the rest of the process from continuing. Need to figure out why it's not deploying automatically.

RothAndrew · 2021-12-09T23:18:00Z

I changed the name of the file from flux_generated.yaml to abc.yaml and it deployed fine, so it must be something about the name of the file. I'll push a commit that changes the name of the generated file to something else that works.

jeff-mccoy · 2021-12-09T23:25:16Z

that's super odd....

RothAndrew · 2021-12-09T23:29:51Z

flux_generated.yaml: doesn't work

flux-generated.yaml: works

🤯 🤯 🤯

RothAndrew · 2021-12-09T23:38:22Z

/test all

Makefile

RothAndrew · 2021-12-09T23:52:23Z

/test all

jeff-mccoy · 2021-12-10T03:42:43Z

/test all

jeff-mccoy · 2021-12-10T04:05:41Z

hmm I thought we resolved these terraform versions issues already...also why do they keep popping up? I've never seen so many issues with terraform versions before. 😭

RothAndrew · 2021-12-10T04:32:50Z

We can loosen up the version constraint. It is very restrictive right now.

RothAndrew · 2021-12-11T00:44:51Z

/test all

…logic

RothAndrew · 2021-12-11T00:55:43Z

As soon a Jeff pushes up the fix to remove --generate and not need it I think I'll be ready to approve and merge

RothAndrew · 2021-12-11T00:57:22Z

I'm not sure how it's possible that the E2E tests are passing if we aren't using --generate but Jeff's fix hasn't been pushed up yet. They should be stuck waiting for a nonexistent user to hit the enter key

jeff-mccoy · 2021-12-11T00:59:54Z

/test all

RothAndrew · 2021-12-11T01:10:14Z

I'm still being prompted to choose whether to generate the CA or to import one when running ./zarf init --confirm --components management,gitops-service --host 127.0.0.1. I'm completely at a loss why I'm seeing it on my machine but the E2E test isn't.

jeff-mccoy · 2021-12-11T01:11:27Z

I'm still being prompted to choose whether to generate the CA or to import one when running ./zarf init --confirm --components management,gitops-service --host 127.0.0.1. I'm completely at a loss why I'm seeing it on my machine but the E2E test isn't.

Okay let me look at this

RothAndrew · 2021-12-11T01:11:40Z

I thought maybe I was still on an older commit somehow but there's the "press tab to see your valid values" stuff...

jeff-mccoy · 2021-12-11T01:11:45Z

oh weird its a race condition

jeff-mccoy · 2021-12-11T01:12:01Z

If you let it sit there it will keep running

jeff-mccoy · 2021-12-11T01:12:44Z

crap I remember now, I was supposed to put a top-level confirm box but forgot to.... ugh

jeff-mccoy · 2021-12-11T01:26:13Z

/test all

jeff-mccoy · 2021-12-11T01:39:50Z

/test all

jeff-mccoy · 2021-12-11T02:04:44Z

........and now registry1 auth is down lol

RothAndrew · 2021-12-11T02:07:00Z

Yep, that was my signal to quit for the night :D

jeff-mccoy · 2021-12-11T03:41:34Z

Okay I'm done with the PR now, all tests pass and I set it to squash/automerge whenever you guys get back to work. I'm going to pick back up my other work later tonight and reconcile against this branch even if it doesn't get merged until next week. Thanks for all the coordination, #teamworkMakesTheDreamWork.

) ### Breaking Changes: * `localhost` is no longer a valid option for cluster ingress when initializing a zarf cluster. Instead you have to use a `127.0.0.1` or some other local ip found via `ifconfig` ### Fixes: * No longer depends on 127.0.0.1 local bindings for the registry / gitops service * should fix #193 * Resolve outstanding issues with image hostname swapping and * fixes #18 * fixes #44 * fixes #194 ### Features: * Adds `before` and `after` script options when defining a `zarf.yaml` with an optional retry flag * Add symlink to ZarfFile for creating links to places files * Add template boolean to ZarfFile to allow injection of zarf variables into text files * Adds a new `zarf tool` command to print out config schema and commit the output to the repo (will need to make a git hook or something later on) * Changes `zarf destroy` command to run any script that starts with `zarf-clean` instead of only running the k3s-remove script * Add new ZarfState and `.zarf-state.yaml` for persisting host information from `zarf init` to `zarf package deploy` * Remove all hard-coded logic for k3s install, now uses only standard zarf component features like everything else * Add user prompt with host/IP address suggestions for ingress #### Misc: * Upgrades k3s from v1.21.2 to v1.21.6 * Adds optional regex filter for when performing RecursiveFileList() * Adds more description to the components in zarf.yaml * Renames type ZarfConfig to ZarfPackage in the config pkg * Handful of general code organizing changes (moving yaml related functions to the `...../utils/yaml.go`, etc.) * Expose execCommand() with stdout control * Move traefik to standalone component and drop the internal k3s install of traefik * Use the airgap tarball of K3s instead of manually listing images * Cleanup init prompt logic Signed-off-by: Jeff McCoy <[email protected]>

initial commit of insanity

f1b4865

jeff-mccoy changed the title ~~Extract K8s unrelated code fixes/features out of native apply work~~ Extract unrelated code fixes/features out of k8s native apply work Dec 9, 2021

allow localhost since cthat is only a native apply issue

3c66e0d

YrrepNoj reviewed Dec 9, 2021

View reviewed changes

cli/internal/images/push.go Show resolved Hide resolved

Add more details to image SwapHost function

cf0cce1

block the use of localhost but not 127.0.0.1

f609777

YrrepNoj reviewed Dec 9, 2021

View reviewed changes

cli/cmd/initialize.go Outdated Show resolved Hide resolved

RothAndrew marked this pull request as ready for review December 9, 2021 23:18

Change name of generated flux manifest so K3s will deploy it properly

c83765c

RothAndrew reviewed Dec 9, 2021

View reviewed changes

Makefile Outdated Show resolved Hide resolved

In CI, install the shasum command before trying to build

888671c

revert shasum change, add mac note

b8ffc6d

who needs a tf version anyway???

5cc04cc

Remove --generate everywhere

f6e0893

jeff-mccoy added 2 commits December 11, 2021 00:52

fix x509 generation bug

d4a9032

introduce ip/host suggestions for tls options and cleanup tls prompt …

8a1070c

…logic

cleanup confirm flow for init

30adce2

add missing --confirm to e2e tests

e461b2f

jeff-mccoy requested review from RothAndrew and YrrepNoj December 11, 2021 02:55

jeff-mccoy enabled auto-merge (squash) December 11, 2021 03:37

YrrepNoj approved these changes Dec 11, 2021

View reviewed changes

jeff-mccoy merged commit b2c7a26 into master Dec 11, 2021

jeff-mccoy deleted the extract-new-features-from-native-apply-work branch December 11, 2021 03:46

Extract unrelated code fixes/features out of k8s native apply work #196

Extract unrelated code fixes/features out of k8s native apply work #196

Uh oh!

Conversation

jeff-mccoy commented Dec 9, 2021 • edited by RothAndrew Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking Changes:

Fixes:

Features:

Misc:

Uh oh!

Uh oh!

RothAndrew commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YrrepNoj commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeff-mccoy commented Dec 9, 2021

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

jeff-mccoy commented Dec 9, 2021

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

jeff-mccoy commented Dec 9, 2021

Uh oh!

jeff-mccoy commented Dec 9, 2021

Uh oh!

Uh oh!

RothAndrew commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

jeff-mccoy commented Dec 9, 2021

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

Uh oh!

RothAndrew commented Dec 9, 2021

Uh oh!

jeff-mccoy commented Dec 10, 2021

Uh oh!

jeff-mccoy commented Dec 10, 2021

Uh oh!

RothAndrew commented Dec 10, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

Uh oh!

RothAndrew commented Dec 11, 2021

Uh oh!

jeff-mccoy commented Dec 11, 2021

jeff-mccoy commented Dec 9, 2021 •

edited by RothAndrew

Loading

RothAndrew commented Dec 9, 2021 •

edited

Loading

YrrepNoj commented Dec 9, 2021 •

edited

Loading

RothAndrew commented Dec 9, 2021 •

edited

Loading