Skip to content

Commit

Permalink
Extra error handling around loading remote configuration data. (#229)
Browse files Browse the repository at this point in the history
* Missing configuration value should be considered 'stale'.

* Handle non-200 responses from the cli-config service.

* Ensure in-memory config data structure contains values.

* Typo.

* Give context around non-200 error.

* Update cmd/fastly/main.go

Co-authored-by: Patrick Hamann <[email protected]>

* Update pkg/config/data.go

Co-authored-by: Patrick Hamann <[email protected]>

Co-authored-by: Patrick Hamann <[email protected]>
  • Loading branch information
Integralist and phamann authored Mar 22, 2021
1 parent 529a303 commit af2f052
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 1 deletion.
30 changes: 30 additions & 0 deletions cmd/fastly/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,36 @@ func main() {
}
}

// We have seen a situation where loading data from the remote
// config endpoint has caused a user to end up with a config in the
// non-legacy format but with empty values.
//
// It's unclear how this happens and so as a temporary measure we'll check if
// the in-memory data structure is missing a specific value that's set by the
// CLI, and if so we'll know something bad has happened because at this point
// we expect the data structure to have a non-empty string value.
//
// If we discover we're in that scenario we'll attempt to re-load the
// configuration from the remote endpoint.
if file.CLI.LastChecked == "" {
if verboseOutput {
text.Warning(out, `
There was a problem loading the compatibility and versioning information for the Fastly CLI.
The operation will be retried as this configuration is required.
`)
text.Break(out)
}

err := file.Load(config.RemoteEndpoint, httpClient)
if err != nil {
errors.RemediationError{
Inner: err,
Remediation: errors.NetworkRemediation,
}.Print(os.Stderr)
os.Exit(1)
}
}

// When the local configuration file is stale we'll need to acquire the
// latest version and write that back to disk. To ensure the CLI program
// doesn't complete before the write has finished, we block via a channel.
Expand Down
4 changes: 3 additions & 1 deletion pkg/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
func Stale(lastVersionCheck string, dur string) bool {
d, err := time.ParseDuration("-" + dur)
if err != nil {
return false
// if there is no duration provided, then we should presume the loading of
// remote configuration failed and that we should retry that operation.
return true
}

if t, _ := time.Parse(time.RFC3339, lastVersionCheck); !t.Before(time.Now().Add(d)) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ func (f *File) Load(configEndpoint string, httpClient api.HTTPClient) error {
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return fmt.Errorf("fetching remote configuration: expected '200 OK', received '%s'", resp.Status)
}

err = toml.NewDecoder(resp.Body).Decode(f)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions pkg/update/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,12 @@ func TestCheckAsync(t *testing.T) {
file: config.File{
CLI: config.CLI{
LastChecked: time.Now().Add(-4 * time.Hour).Format(time.RFC3339),
TTL: "5m",
},
},
currentVersion: "0.0.1",
versioner: mock.Versioner{Version: "0.0.2"},
wantOutput: "\nA new version of the Fastly CLI is available.\nCurrent version: 0.0.1\nLatest version: 0.0.2\nRun `fastly update` to get the latest version.\n\n",
},
} {
t.Run(testcase.name, func(t *testing.T) {
Expand Down

0 comments on commit af2f052

Please sign in to comment.