diff --git a/api/types/app.go b/api/types/app.go index 38dd917840c17..8ee97ff53799e 100644 --- a/api/types/app.go +++ b/api/types/app.go @@ -59,6 +59,8 @@ type Application interface { SetURI(string) // GetPublicAddr returns the app public address. GetPublicAddr() string + // SetPublicAddr sets the app public address. + SetPublicAddr(s string) // GetInsecureSkipVerify returns the app insecure setting. GetInsecureSkipVerify() bool // GetRewrite returns the app rewrite configuration. @@ -249,6 +251,11 @@ func (a *AppV3) GetPublicAddr() string { return a.Spec.PublicAddr } +// SetPublicAddr sets the app public address. +func (a *AppV3) SetPublicAddr(addr string) { + a.Spec.PublicAddr = addr +} + // GetInsecureSkipVerify returns the app insecure setting. func (a *AppV3) GetInsecureSkipVerify() bool { return a.Spec.InsecureSkipVerify diff --git a/lib/srv/app/watcher.go b/lib/srv/app/watcher.go index 1e7b50e4707f2..509ebea87511a 100644 --- a/lib/srv/app/watcher.go +++ b/lib/srv/app/watcher.go @@ -88,11 +88,20 @@ func (s *Server) startResourceWatcher(ctx context.Context) (*services.GenericWat for { select { case apps := <-watcher.ResourcesC: - appsWithAddr := make(types.Apps, 0, len(apps)) for _, app := range apps { - appsWithAddr = append(appsWithAddr, s.guessPublicAddr(app)) + if app.GetPublicAddr() == "" { + pubAddr, err := FindPublicAddr(s.c.AccessPoint, app.GetPublicAddr(), app.GetName()) + if err == nil { + app.SetPublicAddr(pubAddr) + } else { + s.log.ErrorContext(s.closeContext, "Unable to find public address for app, leaving empty", + "app_name", app.GetName(), + "error", err, + ) + } + } } - s.monitoredApps.setResources(appsWithAddr) + s.monitoredApps.setResources(apps) select { case s.reconcileCh <- struct{}{}: case <-ctx.Done(): @@ -107,24 +116,6 @@ func (s *Server) startResourceWatcher(ctx context.Context) (*services.GenericWat return watcher, nil } -// guessPublicAddr will guess PublicAddr for given application if it is missing, based on proxy information and app name. -func (s *Server) guessPublicAddr(app types.Application) types.Application { - if app.GetPublicAddr() != "" { - return app - } - appCopy := app.Copy() - pubAddr, err := FindPublicAddr(s.c.AccessPoint, app.GetPublicAddr(), app.GetName()) - if err == nil { - appCopy.Spec.PublicAddr = pubAddr - } else { - s.log.ErrorContext(s.closeContext, "Unable to find public address for app, leaving empty", - "app_name", app.GetName(), - "error", err, - ) - } - return appCopy -} - // FindPublicAddrClient is a client used for finding public addresses. type FindPublicAddrClient interface { // GetProxies returns a list of proxy servers registered in the cluster diff --git a/lib/srv/app/watcher_test.go b/lib/srv/app/watcher_test.go index a319d1b25cb58..3cd19b8cf4bf0 100644 --- a/lib/srv/app/watcher_test.go +++ b/lib/srv/app/watcher_test.go @@ -19,13 +19,13 @@ package app import ( - "context" + "cmp" "maps" - "sort" + "slices" "testing" "time" - "github.com/google/go-cmp/cmp" + gocmp "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" @@ -36,7 +36,7 @@ import ( // TestWatcher verifies that app agent properly detects and applies // changes to application resources. func TestWatcher(t *testing.T) { - ctx := context.Background() + ctx := t.Context() // Make a static configuration app. app0, err := makeStaticApp("app0", nil) @@ -60,11 +60,27 @@ func TestWatcher(t *testing.T) { }, }) + // Create a single Proxy with a PublicAddr to exercise that + // apps without a PublicAddr automatically get one specified + // by the watcher. + require.NoError(t, s.authServer.AuthServer.UpsertProxy(t.Context(), &types.ServerV2{ + Kind: types.KindProxy, + Version: types.V2, + Metadata: types.Metadata{ + Name: "FakeProxy", + }, + Spec: types.ServerSpecV2{ + PublicAddrs: []string{"test.example.com"}, + }, + })) + // Only app0 should be registered initially. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -77,11 +93,18 @@ func TestWatcher(t *testing.T) { err = s.authServer.AuthServer.CreateApp(ctx, app1) require.NoError(t, err) + // Set the PublicAddr _after_ creating the app. The watched apps will + // automatically have the address set if empty. In order for the comparisons + // below to pass this needs to be set on app1. + app1.SetPublicAddr("app1.test.example.com") + // It should be registered. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app1}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app1}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -97,8 +120,10 @@ func TestWatcher(t *testing.T) { // It should not be registered, old app0 should remain. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app1}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app1}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -108,14 +133,21 @@ func TestWatcher(t *testing.T) { // Create app with label group=b. app2, err := makeDynamicApp("app2", map[string]string{"group": "b"}) require.NoError(t, err) + + // Set the PublicAddr _before_ creating the app. The watcher should + // not modify apps with an already specified PublicAddr. + app2.SetPublicAddr("app2.some.other.addr.example.com") + err = s.authServer.AuthServer.CreateApp(ctx, app2) require.NoError(t, err) // It shouldn't be registered. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app1}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app1}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -130,8 +162,10 @@ func TestWatcher(t *testing.T) { // Both should be registered now. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app1, app2}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app1, app2}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -146,8 +180,10 @@ func TestWatcher(t *testing.T) { // app2 should get updated. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app1, app2}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app1, app2}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -162,8 +198,10 @@ func TestWatcher(t *testing.T) { // Only app0 and app2 should remain registered. select { case a := <-reconcileCh: - sort.Sort(a) - require.Empty(t, cmp.Diff(types.Apps{app0, app2}, a, + slices.SortFunc(a, func(a, b types.Application) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + require.Empty(t, gocmp.Diff(types.Apps{app0, app2}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second): @@ -177,7 +215,7 @@ func TestWatcher(t *testing.T) { // Only static app should remain. select { case a := <-reconcileCh: - require.Empty(t, cmp.Diff(types.Apps{app0}, a, + require.Empty(t, gocmp.Diff(types.Apps{app0}, a, cmpopts.IgnoreFields(types.Metadata{}, "Revision"), )) case <-time.After(time.Second):