Fix etcd backup/restore test and add guardrail for etcd-snapshot

[manuelbuil]

Proposed Changes

This PR does two things:
1 - Fixes the e2e test doing backup/restore from a snapshot. It was failing with:

FATA[0000] cannot perform cluster-reset while server URL is set - remove server from configuration before resetting

When comparing the steps it was following and the documentation, it was indeed wrong. The rest of the servers don't have to run --cluster-reset. When removing the step Resets non bootstrap nodes, the e2e test works again

2 - I must confess that I wasted an hour trying to use k3s etcd-snapshot in a k3s cluster that was not using etcd. When doing that, the error we get is super confusing:

FATA[0000] see server log for details: Unauthorized

However, there is nothing in the logs. The problem is that we are treating a 404 (url does not exist) as Unauthorized https://github.com/k3s-io/k3s/blob/master/pkg/server/router.go#L87. We have been doing that for a long time, so it is probably risky to change that. Therefore, I decided to check if kine.sock exists and db/etcd/config not in the dataDir. If that's the case, we print K3s is not deployed with an etcd datastore

Types of Changes

Test fix + guardrail

Verification

1 - Run the e2e test
2 - Run any k3s etcd-snapshot command without etcd as datastore

Testing

Linked Issues

User-Facing Change

Further Comments

[codecov]

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 42.44%. Comparing base (b93fd98) to head (cd5478f).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/cluster/managed.go	72.72%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11314      +/-   ##
==========================================
- Coverage   46.89%   42.44%   -4.45%     
==========================================
  Files         179      179              
  Lines       18587    18610      +23     
==========================================
- Hits         8716     7899     -817     
- Misses       8518     9505     +987     
+ Partials     1353     1206     -147     

Flag	Coverage Δ
e2etests	34.33% <72.72%> (-7.61%)	⬇️
inttests	18.81% <0.00%> (-15.85%)	⬇️
unittests	13.82% <0.00%> (+0.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[brandond]

The problem is that we are treating a 404 (url does not exist) as Unauthorized https://github.com/k3s-io/k3s/blob/master/pkg/server/router.go#L87.

That's not what's happening here. The NotFoundHandler field is perhaps poorly named, but it is just how mux handles chaining request handlers. If a router handler does not find a matching route, it hands it off to the next handler. In this case you'll notice that section of code setting up a long chain of routers where each router in the chain sets the previous one as its NotFound handler, and the final router is whats returned. Because etcd isn't present to install any routes that match the snapshot path, you end up getting passed all the way through to the apiserver, which is what generates the Unauthorized response.

We do have other handlers that return proper errors if etcd isn't enabled, for example:

k3s/pkg/server/router.go

Lines 118 to 126 in b93fd98

	func bootstrapHandler(runtime *config.ControlRuntime) http.Handler {
	if runtime.HTTPBootstrap {
	return bootstrap.Handler(&runtime.ControlRuntimeBootstrap)
	}
	return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
	logrus.Warnf("Received HTTP bootstrap request from %s, but embedded etcd is not enabled.", req.RemoteAddr)
	util.SendError(errors.New("etcd disabled"), resp, req, http.StatusBadRequest)
	})
	}

Here's where the snapshot handlers are registered:

• k3s/pkg/cluster/managed.go

  Lines 94 to 103 in b93fd98

  	// registerDBHandlers registers managed-datastore-specific callbacks, and installs additional HTTP route handlers.
  	// Note that for etcd, controllers only run on nodes with a local apiserver, in order to provide stable external
  	// management of etcd cluster membership without being disrupted when a member is removed from the cluster.
  	func (c *Cluster) registerDBHandlers(handler http.Handler) (http.Handler, error) {
  	if c.managedDB == nil {
  	return handler, nil
  	}
  	return c.managedDB.Register(handler)
  	}

• k3s/pkg/etcd/etcd.go

  Lines 632 to 633 in b93fd98

  	// Register adds db info routes for the http request handler, and registers cluster controller callbacks
  	func (e *ETCD) Register(handler http.Handler) (http.Handler, error) {

• k3s/pkg/etcd/etcd.go

  Lines 696 to 697 in b93fd98

  	// handler wraps the handler with routes for database info
  	func (e *ETCD) handler(next http.Handler) http.Handler {

We do not need to worry about returning 404s for the snapshot handlers if etcd isn't enabled, since that's not what's currently happening. Nor should the CLI be checking for etcd datastore files - request validation should happen on the server side.

You could probably do a couple different things:

• Add a default handler in router.go that matches /db/ and sends an appropriate error - counting on the fact that the etcd router is later in the chain and the requests won't ever make it to this handler if etcd is running.
• Add a default handler in managed.go that is only added if etcd isn't enabled. This is probably technically safer, since it would avoid installing the error handler if etcd is running.

[manuelbuil]

• Add a default handler in managed.go that is only added if etcd isn't enabled. This is probably technically safer, since it would avoid installing the error handler if etcd is running.

Thanks for the explanation and the suggestion. I took this path, I also agree it is safer

[brandond]

whitespace nit

[cwayne18]

/trivy

[github-actions]

❌ Trivy scan action failed, check logs ❌

[github-actions]

❌ Trivy scan action failed, check logs ❌

[manuelbuil]

❌ Trivy scan action failed, check logs ❌

2024-11-21T18:45:05Z	ERROR	[vulndb] Failed to download artifact	repo="ghcr.io/aquasecurity/trivy-db:2" err="OCI repository error: 1 error occurred:\n\t* GET https://ghcr.io/v2/aquasecurity/trivy-db/manifests/2: TOOMANYREQUESTS: retry-after: 324.606µs, allowed: 44000/minute\n\n"

scale up trivy!

[github-actions]

🌟 No High or Critical CVEs Found 🌟