Improve readme steps for installation of apps

[Elias-elastisys]

Warning

This is a public repository, ensure not to disclose:

• personal data beyond what is necessary for interacting with this pull request, nor
• business confidential information, such as customer names.

What kind of PR is this?

Required: Mark one of the following that is applicable:

• kind/feature
• kind/improvement
• kind/deprecation
• kind/documentation
• kind/clean-up
• kind/bug
• kind/other

Optional: Mark one or more of the following that are applicable:

Important

Breaking changes should be marked kind/admin-change or kind/dev-change depending on type
Critical security fixes should be marked with kind/security

• kind/admin-change
• kind/dev-change
• kind/security
• kind/adr

What does this PR do / why do we need this PR?

Tries to improve the readme so that it is possible to install apps solely based on it.

Decided in https://github.com/elastisys/ck8s-arch/issues/215

Information to reviewers

Checklist

• Proper commit message prefix on all commits
• Change checks:
  • The change is transparent
  • The change is disruptive
  • The change requires no migration steps
  • The change requires migration steps
  • The change upgrades CRDs
  • The change updates the config and the schema
• Documentation checks:
  • The public documentation required no updates
  • The public documentation required an update - [link to change](set-me)
• Metrics checks:
  • The metrics are still exposed and present in Grafana after the change
  • The metrics names didn't change (Grafana dashboards and Prometheus alerts are not affected)
  • The metrics names did change (Grafana dashboards and Prometheus alerts were fixed)
• Logs checks:
  • The logs do not show any errors after the change
• Pod Security Policy checks:
  • Any changed pod is covered by Pod Security Admission
  • Any changed pod is covered by Gatekeeper Pod Security Policies
  • The change does not cause any pods to be blocked by Pod Security Admission or Policies
• Network Policy checks:
  • Any changed pod is covered by Network Policies
  • The change does not cause any dropped packets in the NetworkPolicy Dashboard
• Audit checks:
  • The change does not cause any unnecessary Kubernetes audit events
  • The change requires changes to Kubernetes audit policy
• Falco checks:
  • The change does not cause any alerts to be generated by Falco
• Bug checks:
  • The bug fix is covered by regression tests

[simonklb]

No issues found in the README but the change in the update-ips script looks dangerous. :o

[simonklb]

Will this not do the opposite, i.e. allow all IPs? This function is used to resolve a domain name to an IP so that it can be added to the allow list, no?

[Elias-elastisys]

This eventually sets the CIDR in the NP to 0.0.0.0/32, i.e. just the 0.0.0.0 ip which would never be a real IP meaning it blocks everything.

[Elias-elastisys]

The process_ips_to_cidrs seems to be the one adding the /32

[simonklb]

Ah of course 🤦

I'm still a bit worried that we might at some point rewrite this and not all IPs go through that same IP to CIDR transformation function.

Also, why would you want it not to fail if the domain can't be resolved? Is it not better to become aware of it?

[Elias-elastisys]

So, when you use a LoadBalancer Service for ingress-nginx you can't create the domains before installing ingress-nginx. And before you could not run update-ips since it would fail and the schema validation would complain if you tried to apply anything. This seemed like a better approach then having to manually set all NPs to something first. Not perfect but I'm not sure if there is a better solution.

Although I just realized I forgot to mention you would have to re-run update-ips again after setting up the DNS.

[aarnq]

Could it not be left emptied instead to disallow everything that way? 🤔
(IIRC the associated rules shouldn't be rendered in that case, because empty netpols allow everything, but we have a split between the old and new netpol still.)

[simonklb]

Maybe split the script change and the documentation changes into separate PRs, both good for changelog purposes and review purposes

[Elias-elastisys]

Could it not be left emptied instead to disallow everything that way? 🤔

If I leave it empty instead the CIDR ends up as just /32 which fails in the apply

[cristiklein]

Overall, I like the improvements, but I'm missing a section which discusses "the forest" before discussing "the trees".

[cristiklein]

I'm missing a section giving an overview of Apps. This is stuff that we already know and have in our DNA, which is why it's so hard to write it. Someone completely new to the project should first learn that:

• Welkin follows the "configuration as code" principle and expects all configuration to be in a so-called configuration folder.
• Welkin strongly recommends to have this configuration folder in .git. Apps may be a submodule, so that the configuration folder is the source of truth for how an environment is set up.
• Welkin Apps' functionality is accessed threw the ./bin/ck8s command-line tool.

Makes sense?

[Elias-elastisys]

Pushed a change. Is that satisfactory? Hard to know where to exactly place it.

No longer relevant since script change is moved to separate PR

[anders-elastisys]

I am wondering if this README somewhere and somehow should state that you might not be able to run commands for wc if you have not installed Dex in sc first, since we usually assume for Welkin that wc is configured to point to Dex in sc 🤔

[anders-elastisys]

We should maybe include that the nodes should be labelled with the elastisys.io/node-group based on what group it belongs to (e.g. worker or control-plane) for our node capacity alerts to work properly