client: unexport the watcher type and update NewUpdater

The watcher semantics are still useful for plumbing, but it's hard to use
correctly, so unexport it.  Now that watcher is unexported, move its tests from
a separate package into an internal package test.

It is now no longer necessary to have the "watcher" method, so remove it.

An Updater now no longer accepts a watcher (which is unexported), but instead
takes a *Store and the desired secret name directly. Rework the constructor to
allow reporting an error.

Add a StaticUpdater constructor to make an Updater that vends a static value,
analogous to StaticSecret.

client/setec/fields.go

@@ -166,7 +166,7 @@ func (f fieldInfo) apply(ctx context.Context, s *Store, fullName string) error {
 	}
 
 	if f.vtype == watcherType {
-		w, err := s.LookupWatcher(ctx, fullName)
+		w, err := s.lookupWatcher(ctx, fullName)
 		if err != nil {
 			return err
 		}
@@ -198,7 +198,7 @@ var (
 	bytesType   = reflect.TypeOf([]byte(nil))
 	secretType  = reflect.TypeOf(Secret(nil))
 	stringType  = reflect.TypeOf(string(""))
-	watcherType = reflect.TypeOf(Watcher{})
+	watcherType = reflect.TypeOf(watcher{})
 	binaryType  = reflect.TypeOf((*encoding.BinaryUnmarshaler)(nil)).Elem()
 )
 

client/setec/fields_test.go

@@ -66,15 +66,14 @@ func TestFields(t *testing.T) {
 	// Verify that if we parse secrets with a store enabled, we correctly plumb
 	// the values from the service into the tagged fields.
 	type testTarget struct {
-		A  string        `setec:"apple"`
-		B  binValue      `setec:"bin-value"`
-		BP *binValue     `setec:"bin-value-ptr"`
-		P  []byte        `setec:"pear"`
-		L  setec.Secret  `setec:"plum"`
-		C  setec.Watcher `setec:"cherry"`
-		X  string        // untagged, not affected
-		J  testObj       `setec:"object-value,json"`
-		Z  int           `setec:"int-value,json"`
+		A  string       `setec:"apple"`
+		B  binValue     `setec:"bin-value"`
+		BP *binValue    `setec:"bin-value-ptr"`
+		P  []byte       `setec:"pear"`
+		L  setec.Secret `setec:"plum"`
+		X  string       // untagged, not affected
+		J  testObj      `setec:"object-value,json"`
+		Z  int          `setec:"int-value,json"`
 	}
 	var obj testTarget
 
@@ -98,7 +97,7 @@ func TestFields(t *testing.T) {
 	// Check that secret names respect prefixing.
 	if diff := cmp.Diff(f.Secrets(), []string{
 		"test/apple", "test/bin-value", "test/bin-value-ptr",
-		"test/pear", "test/plum", "test/cherry",
+		"test/pear", "test/plum",
 		"test/object-value", "test/int-value",
 	}); diff != "" {
 		t.Errorf("Prefixed secret names (-got, +want):\n%s", diff)
@@ -110,7 +109,7 @@ func TestFields(t *testing.T) {
 	}
 
 	// Don't try to compare complex plumbing; see below.
-	opt := cmpopts.IgnoreFields(testTarget{}, "L", "C")
+	opt := cmpopts.IgnoreFields(testTarget{}, "L")
 	if diff := cmp.Diff(obj, testTarget{
 		A:  secrets["apple"],
 		B:  binValue{"kumquat", "quince"},
@@ -126,9 +125,6 @@ func TestFields(t *testing.T) {
 	if got, want := string(obj.L.Get()), secrets["plum"]; got != want {
 		t.Errorf("Secret field: got %q, want %q", got, want)
 	}
-	if got, want := string(obj.C.Get()), secrets["cherry"]; got != want {
-		t.Errorf("Secret field: got %q, want %q", got, want)
-	}
 }
 
 func TestParseErrors(t *testing.T) {

client/setec/internal_test.go

@@ -0,0 +1,78 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package setec
+
+import (
+	"context"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/tailscale/setec/setectest"
+)
+
+func TestWatcher(t *testing.T) {
+	d := setectest.NewDB(t, nil)
+	d.MustPut(d.Superuser, "green", "eggs and ham") // active
+	v2 := d.MustPut(d.Superuser, "green", "grow the rushes oh")
+
+	ts := setectest.NewServer(t, d, nil)
+	hs := httptest.NewServer(ts.Mux)
+	defer hs.Close()
+
+	ctx := context.Background()
+	cli := Client{Server: hs.URL, DoHTTP: hs.Client().Do}
+
+	pollTicker := setectest.NewFakeTicker()
+	st, err := NewStore(ctx, StoreConfig{
+		Client:     cli,
+		Secrets:    []string{"green"},
+		PollTicker: pollTicker,
+	})
+	if err != nil {
+		t.Fatalf("NewStore: unexpected error: %v", err)
+	}
+	defer st.Close()
+
+	// With lookups disabled, an unknown watcher reports an error.
+	if w, err := st.lookupWatcher(ctx, "nonesuch"); err == nil {
+		t.Errorf("Lookup: got %v, want error", w)
+	}
+
+	// Observe the initial value of the secret.
+	w, err := st.lookupWatcher(ctx, "green")
+	if err != nil {
+		t.Errorf("Initial value: unexpected error: %v", err)
+	} else if got, want := string(w.Get()), "eggs and ham"; got != want {
+		t.Errorf("Initial value: got %q, want %q", got, want)
+	}
+
+	// The secret gets updated...
+	if err := cli.Activate(ctx, "green", v2); err != nil {
+		t.Fatalf("Activate to %v: unexpected error: %v", v2, err)
+	}
+
+	// The next poll occurs...
+	pollTicker.Poll()
+
+	// The watcher should get notified in a timely manner.
+	select {
+	case <-w.Ready():
+		t.Logf("✓ A new version of the secret is available")
+	case <-time.After(5 * time.Second):
+		t.Fatal("Timed out waiting for a watcher update")
+	}
+
+	if got, want := string(w.Get()), "grow the rushes oh"; got != want {
+		t.Errorf("Updated value: got %q, want %q", got, want)
+	}
+
+	// With no updates, the watchers should not appear ready.
+	select {
+	case <-w.Ready():
+		t.Error("Watcher is unexpectedly ready after no update")
+	case <-time.After(100 * time.Millisecond):
+		// OK
+	}
+}