cmd/tailscale/cli: use optimistic concurrency control on SetServeConfig
This PR uses the etag/if-match pattern to ensure multiple calls to SetServeConfig are synchronized. It currently errors out and asks the user to retry but we can add occ retries as a follow up. Updates #8489 Signed-off-by: Marwan Sulaiman <marwan@tailscale.com>
This commit is contained in:
parent
6e66e5beeb
commit
3421784e37
|
@ -140,6 +140,10 @@ func (lc *LocalClient) doLocalRequestNiceError(req *http.Request) (*http.Respons
|
|||
all, _ := io.ReadAll(res.Body)
|
||||
return nil, &AccessDeniedError{errors.New(errorMessageFromBody(all))}
|
||||
}
|
||||
if res.StatusCode == http.StatusPreconditionFailed {
|
||||
all, _ := io.ReadAll(res.Body)
|
||||
return nil, &PreconditionsFailedError{errors.New(errorMessageFromBody(all))}
|
||||
}
|
||||
return res, nil
|
||||
}
|
||||
if ue, ok := err.(*url.Error); ok {
|
||||
|
@ -170,6 +174,24 @@ func IsAccessDeniedError(err error) bool {
|
|||
return errors.As(err, &ae)
|
||||
}
|
||||
|
||||
// PreconditionsFailedError is returned when the server responds
|
||||
// with an HTTP 412 status code.
|
||||
type PreconditionsFailedError struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (e *PreconditionsFailedError) Error() string {
|
||||
return fmt.Sprintf("Preconditions failed: %v", e.err)
|
||||
}
|
||||
|
||||
func (e *PreconditionsFailedError) Unwrap() error { return e.err }
|
||||
|
||||
// IsPreconditionsFailedError reports whether err is or wraps an PreconditionsFailedError.
|
||||
func IsPreconditionsFailedError(err error) bool {
|
||||
var ae *PreconditionsFailedError
|
||||
return errors.As(err, &ae)
|
||||
}
|
||||
|
||||
// bestError returns either err, or if body contains a valid JSON
|
||||
// object of type errorJSON, its non-empty error body.
|
||||
func bestError(err error, body []byte) error {
|
||||
|
@ -198,27 +220,42 @@ func SetVersionMismatchHandler(f func(clientVer, serverVer string)) {
|
|||
}
|
||||
|
||||
func (lc *LocalClient) send(ctx context.Context, method, path string, wantStatus int, body io.Reader) ([]byte, error) {
|
||||
slurp, _, err := lc.sendWithHeaders(ctx, method, path, wantStatus, body, nil)
|
||||
return slurp, err
|
||||
}
|
||||
|
||||
func (lc *LocalClient) sendWithHeaders(
|
||||
ctx context.Context,
|
||||
method,
|
||||
path string,
|
||||
wantStatus int,
|
||||
body io.Reader,
|
||||
h http.Header,
|
||||
) ([]byte, http.Header, error) {
|
||||
if jr, ok := body.(jsonReader); ok && jr.err != nil {
|
||||
return nil, jr.err // fail early if there was a JSON marshaling error
|
||||
return nil, nil, jr.err // fail early if there was a JSON marshaling error
|
||||
}
|
||||
req, err := http.NewRequestWithContext(ctx, method, "http://"+apitype.LocalAPIHost+path, body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
if h != nil {
|
||||
req.Header = h
|
||||
}
|
||||
res, err := lc.doLocalRequestNiceError(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
defer res.Body.Close()
|
||||
slurp, err := io.ReadAll(res.Body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
if res.StatusCode != wantStatus {
|
||||
err = fmt.Errorf("%v: %s", res.Status, bytes.TrimSpace(slurp))
|
||||
return nil, bestError(err, slurp)
|
||||
return nil, nil, bestError(err, slurp)
|
||||
}
|
||||
return slurp, nil
|
||||
return slurp, res.Header, nil
|
||||
}
|
||||
|
||||
func (lc *LocalClient) get200(ctx context.Context, path string) ([]byte, error) {
|
||||
|
@ -1093,7 +1130,11 @@ func (lc *LocalClient) NetworkLockSubmitRecoveryAUM(ctx context.Context, aum tka
|
|||
// SetServeConfig sets or replaces the serving settings.
|
||||
// If config is nil, settings are cleared and serving is disabled.
|
||||
func (lc *LocalClient) SetServeConfig(ctx context.Context, config *ipn.ServeConfig) error {
|
||||
_, err := lc.send(ctx, "POST", "/localapi/v0/serve-config", 200, jsonBody(config))
|
||||
h := make(http.Header)
|
||||
if config != nil {
|
||||
h.Set("If-Match", config.ETag)
|
||||
}
|
||||
_, _, err := lc.sendWithHeaders(ctx, "POST", "/localapi/v0/serve-config", 200, jsonBody(config), h)
|
||||
if err != nil {
|
||||
return fmt.Errorf("sending serve config: %w", err)
|
||||
}
|
||||
|
@ -1112,11 +1153,19 @@ func (lc *LocalClient) NetworkLockDisable(ctx context.Context, secret []byte) er
|
|||
//
|
||||
// If the serve config is empty, it returns (nil, nil).
|
||||
func (lc *LocalClient) GetServeConfig(ctx context.Context) (*ipn.ServeConfig, error) {
|
||||
body, err := lc.send(ctx, "GET", "/localapi/v0/serve-config", 200, nil)
|
||||
body, h, err := lc.sendWithHeaders(ctx, "GET", "/localapi/v0/serve-config", 200, nil, nil)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("getting serve config: %w", err)
|
||||
}
|
||||
return getServeConfigFromJSON(body)
|
||||
sc, err := getServeConfigFromJSON(body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if sc == nil {
|
||||
sc = new(ipn.ServeConfig)
|
||||
}
|
||||
sc.ETag = h.Get("Etag")
|
||||
return sc, nil
|
||||
}
|
||||
|
||||
func getServeConfigFromJSON(body []byte) (sc *ipn.ServeConfig, err error) {
|
||||
|
|
|
@ -275,6 +275,9 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
|
|||
}
|
||||
|
||||
if err := e.lc.SetServeConfig(ctx, parentSC); err != nil {
|
||||
if tailscale.IsPreconditionsFailedError(err) {
|
||||
fmt.Fprintln(os.Stderr, "Another client is changing the serve config; please try again.")
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
|
@ -91,6 +91,7 @@ var _ServeConfigCloneNeedsRegeneration = ServeConfig(struct {
|
|||
Web map[HostPort]*WebServerConfig
|
||||
AllowFunnel map[HostPort]bool
|
||||
Foreground map[string]*ServeConfig
|
||||
ETag string
|
||||
}{})
|
||||
|
||||
// Clone makes a deep copy of TCPPortHandler.
|
||||
|
|
|
@ -182,6 +182,7 @@ func (v ServeConfigView) Foreground() views.MapFn[string, *ServeConfig, ServeCon
|
|||
return t.View()
|
||||
})
|
||||
}
|
||||
func (v ServeConfigView) ETag() string { return v.ж.ETag }
|
||||
|
||||
// A compilation failure here means this code must be regenerated, with the command at the top of this file.
|
||||
var _ServeConfigViewNeedsRegeneration = ServeConfig(struct {
|
||||
|
@ -189,6 +190,7 @@ var _ServeConfigViewNeedsRegeneration = ServeConfig(struct {
|
|||
Web map[HostPort]*WebServerConfig
|
||||
AllowFunnel map[HostPort]bool
|
||||
Foreground map[string]*ServeConfig
|
||||
ETag string
|
||||
}{})
|
||||
|
||||
// View returns a readonly view of TCPPortHandler.
|
||||
|
|
|
@ -44,6 +44,12 @@ type ServeConfig struct {
|
|||
// of either the client or the LocalBackend does not expose ports
|
||||
// that users are not aware of.
|
||||
Foreground map[string]*ServeConfig `json:",omitempty"`
|
||||
|
||||
// ETag is the checksum of the serve config that's populated
|
||||
// by the LocalClient through the HTTP ETag header during a
|
||||
// GetServeConfig request and is translated to an If-Match header
|
||||
// during a SetServeConfig request.
|
||||
ETag string `json:"-"`
|
||||
}
|
||||
|
||||
// HostPort is an SNI name and port number, joined by a colon.
|
||||
|
|
Loading…
Reference in New Issue