-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose client tools auto update for find endpoint #46785
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1516,7 +1516,7 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, p httprouter.Para | |
if err != nil { | ||
return nil, trace.Wrap(err) | ||
} | ||
return webclient.PingResponse{ | ||
response := webclient.PingResponse{ | ||
Auth: webclient.AuthenticationSettings{ | ||
// Nodes need the signature algorithm suite when joining to generate | ||
// keys with the correct algorithm. | ||
|
@@ -1526,7 +1526,25 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, p httprouter.Para | |
ServerVersion: teleport.Version, | ||
MinClientVersion: teleport.MinClientVersion, | ||
ClusterName: h.auth.clusterName, | ||
}, nil | ||
} | ||
|
||
autoUpdateConfig, err := h.cfg.AccessPoint.GetAutoUpdateConfig(r.Context()) | ||
// TODO(vapopov) DELETE IN v18.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: make the deletion note clearer that its only the not found and not implemented checks that should be removed in v18 |
||
if err != nil && !trace.IsNotFound(err) && !trace.IsNotImplemented(err) { | ||
h.logger.ErrorContext(r.Context(), "failed to receive AutoUpdateConfig", "error", err) | ||
} else if err == nil { | ||
response.AutoUpdate.ToolsAutoupdate = autoUpdateConfig.GetSpec().GetToolsAutoupdate() | ||
} | ||
|
||
autoUpdateVersion, err := h.cfg.AccessPoint.GetAutoUpdateVersion(r.Context()) | ||
// TODO(vapopov) DELETE IN v18.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: make the deletion note clearer that its only the not found and not implemented checks that should be removed in v18 |
||
if err != nil && !trace.IsNotFound(err) && !trace.IsNotImplemented(err) { | ||
h.logger.ErrorContext(r.Context(), "failed to receive AutoUpdateVersion", "error", err) | ||
} else if err == nil { | ||
response.AutoUpdate.ToolsVersion = autoUpdateVersion.GetSpec().GetToolsVersion() | ||
} | ||
|
||
return response, nil | ||
} | ||
|
||
func (h *Handler) pingWithConnector(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -29,9 +29,12 @@ import ( | |||||||||
"github.com/stretchr/testify/assert" | ||||||||||
"github.com/stretchr/testify/require" | ||||||||||
|
||||||||||
"github.com/gravitational/teleport" | ||||||||||
"github.com/gravitational/teleport/api/client/webclient" | ||||||||||
"github.com/gravitational/teleport/api/constants" | ||||||||||
autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" | ||||||||||
"github.com/gravitational/teleport/api/types" | ||||||||||
"github.com/gravitational/teleport/api/types/autoupdate" | ||||||||||
"github.com/gravitational/teleport/lib/client" | ||||||||||
"github.com/gravitational/teleport/lib/modules" | ||||||||||
) | ||||||||||
|
@@ -244,5 +247,44 @@ func TestPing_minimalAPI(t *testing.T) { | |||||||||
require.NoError(t, resp.Body.Close()) | ||||||||||
}) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
// TestPing_autoUpdateResources tests that find endpoint return correct data related to auto updates. | ||||||||||
func TestPing_autoUpdateResources(t *testing.T) { | ||||||||||
env := newWebPack(t, 1, func(cfg *proxyConfig) { | ||||||||||
cfg.minimalHandler = true | ||||||||||
}) | ||||||||||
proxy := env.proxies[0] | ||||||||||
|
||||||||||
ctx, cancel := context.WithCancel(context.Background()) | ||||||||||
defer cancel() | ||||||||||
|
||||||||||
// Enable tools auto update. | ||||||||||
config, err := autoupdate.NewAutoUpdateConfig(&autoupdatev1pb.AutoUpdateConfigSpec{ | ||||||||||
ToolsAutoupdate: true, | ||||||||||
}) | ||||||||||
require.NoError(t, err) | ||||||||||
_, err = env.server.Auth().UpsertAutoUpdateConfig(ctx, config) | ||||||||||
require.NoError(t, err) | ||||||||||
|
||||||||||
// Set the client tools version. | ||||||||||
version, err := autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{ | ||||||||||
ToolsVersion: teleport.Version, | ||||||||||
}) | ||||||||||
require.NoError(t, err) | ||||||||||
_, err = env.server.Auth().UpsertAutoUpdateVersion(ctx, version) | ||||||||||
require.NoError(t, err) | ||||||||||
|
||||||||||
req, err := http.NewRequest(http.MethodGet, proxy.newClient(t).Endpoint("webapi", "find"), nil) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: add a test case that covers the default values when none of the auto update configurations are populated or there is an error retrieving the configurations? |
||||||||||
require.NoError(t, err) | ||||||||||
req.Host = proxy.handler.handler.cfg.ProxyPublicAddrs[0].Host() | ||||||||||
resp, err := client.NewInsecureWebClient().Do(req) | ||||||||||
require.NoError(t, err) | ||||||||||
|
||||||||||
pr := &webclient.PingResponse{} | ||||||||||
require.NoError(t, json.NewDecoder(resp.Body).Decode(pr)) | ||||||||||
require.NoError(t, resp.Body.Close()) | ||||||||||
|
||||||||||
assert.True(t, pr.AutoUpdate.ToolsAutoupdate, "tools_autoupdate must be enabled") | ||||||||||
assert.Equal(t, teleport.Version, pr.AutoUpdate.ToolsVersion, "tools_version must be equal to current version") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: decouple the json tags from the message
Suggested change
|
||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The json tag for the
AutoUpdate
field in the PingResponse isauto_update
- we should be consistent with the tag here.