ipn/store/kubestore,kube: fix cert error in admin UI (#16717)

Also adds a test to kube/kubeclient to defend against the error type
returned by the client changing in future.

Fixes tailscale/corp#30855

Change-Id: Id11d4295003e66ad5c29a687f1239333c21226a4

Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>

ipn/store/kubestore/store_kube.go

@@ -9,6 +9,7 @@
 	"fmt"
 	"log"
 	"net"
+	"net/http"
 	"os"
 	"strings"
 	"time"
@@ -203,6 +204,23 @@ func (s *Store) ReadTLSCertAndKey(domain string) (cert, key []byte, err error) {
 			// that wraps ipn.ErrStateNotExist here.
 			return nil, nil, ipn.ErrStateNotExist
 		}
+		st, ok := err.(*kubeapi.Status)
+		if ok && st.Code == http.StatusForbidden && (s.certShareMode == "ro" || s.certShareMode == "rw") {
+			// In cert share mode, we read from a dedicated Secret per domain.
+			// To get here, we already had a cache miss from our in-memory
+			// store. For write replicas, that means it wasn't available on
+			// start and it wasn't written since. For read replicas, that means
+			// it wasn't available on start and it hasn't been reloaded in the
+			// background. So getting a "forbidden" error is an expected
+			// "not found" case where we've been asked for a cert we don't
+			// expect to issue, and so the forbidden error reflects that the
+			// operator didn't assign permission for a Secret for that domain.
+			//
+			// This code path gets triggered by the admin UI's machine page,
+			// which queries for the node's own TLS cert existing via the
+			// "tls-cert-status" c2n API.
+			return nil, nil, ipn.ErrStateNotExist
+		}
 		return nil, nil, fmt.Errorf("getting TLS Secret %q: %w", domain, err)
 	}
 	cert = secret.Data[keyTLSCert]

ipn/store/kubestore/store_kube_test.go

@@ -426,6 +426,13 @@ func TestReadTLSCertAndKey(t *testing.T) {
 			secretGetErr:  &kubeapi.Status{Code: 404},
 			wantErr:       ipn.ErrStateNotExist,
 		},
+		{
+			name:          "cert_share_ro_mode_forbidden",
+			certShareMode: "ro",
+			domain:        testDomain,
+			secretGetErr:  &kubeapi.Status{Code: 403},
+			wantErr:       ipn.ErrStateNotExist,
+		},
 		{
 			name:          "cert_share_ro_mode_empty_cert_in_secret",
 			certShareMode: "ro",

kube/kubeclient/client_test.go

@@ -7,6 +7,9 @@
 	"context"
 	"encoding/json"
 	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -104,6 +107,48 @@ func Test_client_Event(t *testing.T) {
 	}
 }
 
+// TestReturnsKubeStatusError ensures HTTP error codes from the Kubernetes API
+// server can always be extracted by casting the error to the *kubeapi.Status
+// type, as lots of calling code relies on this cast succeeding. Note that
+// transport errors are not expected or required to be of type *kubeapi.Status.
+func TestReturnsKubeStatusError(t *testing.T) {
+	cl := clientForKubeHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		_ = json.NewEncoder(w).Encode(kubeapi.Status{Code: http.StatusForbidden, Message: "test error"})
+	}))
+
+	_, err := cl.GetSecret(t.Context(), "test-secret")
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if st, ok := err.(*kubeapi.Status); !ok || st.Code != http.StatusForbidden {
+		t.Fatalf("expected kubeapi.Status with code %d, got %T: %v", http.StatusForbidden, err, err)
+	}
+}
+
+// clientForKubeHandler creates a client using the externally accessible package
+// API to ensure it's testing behaviour as close to prod as possible. The passed
+// in handler mocks the Kubernetes API server's responses to any HTTP requests
+// made by the client.
+func clientForKubeHandler(t *testing.T, handler http.Handler) Client {
+	t.Helper()
+	tmpDir := t.TempDir()
+	rootPathForTests = tmpDir
+	saDir := filepath.Join(tmpDir, "var", "run", "secrets", "kubernetes.io", "serviceaccount")
+	_ = os.MkdirAll(saDir, 0755)
+	_ = os.WriteFile(filepath.Join(saDir, "token"), []byte("test-token"), 0600)
+	_ = os.WriteFile(filepath.Join(saDir, "namespace"), []byte("test-namespace"), 0600)
+	_ = os.WriteFile(filepath.Join(saDir, "ca.crt"), []byte(ca), 0644)
+	cl, err := New("test-client")
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+	srv := httptest.NewServer(handler)
+	t.Cleanup(srv.Close)
+	cl.SetURL(srv.URL)
+	return cl
+}
+
 // args is a set of values for testing a single call to client.kubeAPIRequest.
 type args struct {
 	// wantsMethod is the expected value of 'method' arg.
@@ -149,3 +194,34 @@ func fakeKubeAPIRequest(t *testing.T, argSets []args) kubeAPIRequestFunc {
 	}
 	return f
 }
+
+const ca = `-----BEGIN CERTIFICATE-----
+MIIFEDCCA3igAwIBAgIRANf5NdPojIfj70wMfJVYUg8wDQYJKoZIhvcNAQELBQAw
+gZ8xHjAcBgNVBAoTFW1rY2VydCBkZXZlbG9wbWVudCBDQTE6MDgGA1UECwwxZnJv
+bWJlcmdlckBzdGFyZHVzdC5sb2NhbCAoTWljaGFlbCBKLiBGcm9tYmVyZ2VyKTFB
+MD8GA1UEAww4bWtjZXJ0IGZyb21iZXJnZXJAc3RhcmR1c3QubG9jYWwgKE1pY2hh
+ZWwgSi4gRnJvbWJlcmdlcikwHhcNMjMwMjA3MjAzNDE4WhcNMzMwMjA3MjAzNDE4
+WjCBnzEeMBwGA1UEChMVbWtjZXJ0IGRldmVsb3BtZW50IENBMTowOAYDVQQLDDFm
+cm9tYmVyZ2VyQHN0YXJkdXN0LmxvY2FsIChNaWNoYWVsIEouIEZyb21iZXJnZXIp
+MUEwPwYDVQQDDDhta2NlcnQgZnJvbWJlcmdlckBzdGFyZHVzdC5sb2NhbCAoTWlj
+aGFlbCBKLiBGcm9tYmVyZ2VyKTCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoC
+ggGBAL5uXNnrZ6dgjcvK0Hc7ZNUIRYEWst9qbO0P9H7le08pJ6d9T2BUWruZtVjk
+Q12msv5/bVWHhVk8dZclI9FLXuMsIrocH8bsoP4wruPMyRyp6EedSKODN51fFSRv
+/jHbS5vzUVAWTYy9qYmd6qL0uhsHCZCCT6gfigamHPUFKM3sHDn5ZHWvySMwcyGl
+AicmPAIkBWqiCZAkB5+WM7+oyRLjmrIalfWIZYxW/rojGLwTfneHv6J5WjVQnpJB
+ayWCzCzaiXukK9MeBWeTOe8UfVN0Engd74/rjLWvjbfC+uZSr6RVkZvs2jANLwPF
+zgzBPHgRPfAhszU1NNAMjnNQ47+OMOTKRt7e6jYzhO5fyO1qVAAvGBqcfpj+JfDk
+cccaUMhUvdiGrhGf1V1tN/PislxvALirzcFipjD01isBKwn0fxRugzvJNrjEo8RA
+RvbcdeKcwex7M0o/Cd0+G2B13gZNOFvR33PmG7iTpp7IUrUKfQg28I83Sp8tMY3s
+ljJSawIDAQABo0UwQzAOBgNVHQ8BAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIB
+ADAdBgNVHQ4EFgQU18qto0Fa56kCi/HwfQuC9ECX7cAwDQYJKoZIhvcNAQELBQAD
+ggGBAAzs96LwZVOsRSlBdQqMo8oMAvs7HgnYbXt8SqaACLX3+kJ3cV/vrCE3iJrW
+ma4CiQbxS/HqsiZjota5m4lYeEevRnUDpXhp+7ugZTiz33Flm1RU99c9UYfQ+919
+ANPAKeqNpoPco/HF5Bz0ocepjcfKQrVZZNTj6noLs8o12FHBLO5976AcF9mqlNfh
+8/F0gDJXq6+x7VT5y8u0rY004XKPRe3CklRt8kpeMiP6mhRyyUehOaHeIbNx8ubi
+Pi44ByN/ueAnuRhF9zYtyZVZZOaSLysJge01tuPXF8rBXGruoJIv35xTTBa9BzaP
+YDOGbGn1ZnajdNagHqCba8vjTLDSpqMvgRj3TFrGHdETA2LDQat38uVxX8gxm68K
+va5Tyv7n+6BQ5YTpJjTPnmSJKaXZrrhdLPvG0OU2TxeEsvbcm5LFQofirOOw86Se
+vzF2cQ94mmHRZiEk0Av3NO0jF93ELDrBCuiccVyEKq6TknuvPQlutCXKDOYSEb8I
+MHctBg==
+-----END CERTIFICATE-----`