From 88013e5f354d6497860b7a4fe57d56444c3913b9 Mon Sep 17 00:00:00 2001 From: Stephen Lowrie Date: Wed, 17 Jun 2020 23:06:36 -0500 Subject: [PATCH 1/2] internal/exec/engine: write empty cache config when not provided When the user-provided config is empty write an empty cache config. Additionally restrict fetching configs to only `fetch` stages. --- internal/exec/engine.go | 73 ++++++++++++++++++++++++++++------------- tests/blackbox_test.go | 8 +++++ tests/filesystem.go | 2 +- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/internal/exec/engine.go b/internal/exec/engine.go index 8e2ad1327..2c2fb62ad 100644 --- a/internal/exec/engine.go +++ b/internal/exec/engine.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "strconv" + "strings" "time" "github.com/coreos/go-systemd/v22/journal" @@ -55,6 +56,12 @@ const ( needNetPath = "/run/ignition/neednet" ) +var ( + emptyConfig = types.Config{ + Ignition: types.Ignition{Version: types.MaxVersion.String()}, + } +) + // Engine represents the entity that fetches and executes a configuration. type Engine struct { ConfigCache string @@ -72,9 +79,7 @@ func (e Engine) Run(stageName string) error { fmt.Fprintf(os.Stderr, "engine incorrectly configured\n") return errors.ErrEngineConfiguration } - baseConfig := types.Config{ - Ignition: types.Ignition{Version: types.MaxVersion.String()}, - } + baseConfig := emptyConfig systemBaseConfig, r, err := system.FetchBaseConfig(e.Logger) e.logReport(r) @@ -108,7 +113,7 @@ func (e Engine) Run(stageName string) error { e.Fetcher.Offline = true } - cfg, err := e.acquireConfig() + cfg, err := e.acquireConfig(stageName) if err == resource.ErrNeedNet && stageName == "fetch-offline" { err = SignalNeedNet() if err != nil { @@ -162,26 +167,45 @@ func logStructuredJournalEntry(config string, src string, isReferenced bool) err return nil } -// acquireConfig returns the configuration, first checking a local cache -// before attempting to fetch it from the provider. -func (e *Engine) acquireConfig() (cfg types.Config, err error) { +// acquireConfig will perform differently based on the stage it is being +// called from. In fetch stages it will attempt to fetch the provider +// config (writing an empty provider config if it is empty). In all other +// stages it will attempt to fetch from the local cache only. +func (e *Engine) acquireConfig(stageName string) (cfg types.Config, err error) { + switch { + case strings.HasPrefix(stageName, "fetch"): + cfg, err = e.acquireProviderConfig() + default: + cfg, err = e.acquireCachedConfig() + } + return +} - // First try read the config @ e.ConfigCache. - b, err := ioutil.ReadFile(e.ConfigCache) - if err == nil { - if err = json.Unmarshal(b, &cfg); err != nil { - e.Logger.Crit("failed to parse cached config: %v", err) - } - // Create an http client and fetcher with the timeouts from the cached - // config - err = e.Fetcher.UpdateHttpTimeoutsAndCAs(cfg.Ignition.Timeouts, cfg.Ignition.Security.TLS.CertificateAuthorities, cfg.Ignition.Proxy) - if err != nil { - e.Logger.Crit("failed to update timeouts and CAs for fetcher: %v", err) - return - } +// acquireCachedConfig returns the configuration from a local cache if +// available +func (e *Engine) acquireCachedConfig() (cfg types.Config, err error) { + var b []byte + b, err = ioutil.ReadFile(e.ConfigCache) + if err != nil { return } + if err = json.Unmarshal(b, &cfg); err != nil { + e.Logger.Crit("failed to parse cached config: %v", err) + return + } + // Create an http client and fetcher with the timeouts from the cached + // config + err = e.Fetcher.UpdateHttpTimeoutsAndCAs(cfg.Ignition.Timeouts, cfg.Ignition.Security.TLS.CertificateAuthorities, cfg.Ignition.Proxy) + if err != nil { + e.Logger.Crit("failed to update timeouts and CAs for fetcher: %v", err) + return + } + return +} +// acquireProviderConfig attempts to fetch the configuration from the +// provider. +func (e *Engine) acquireProviderConfig() (cfg types.Config, err error) { // Create a new http client and fetcher with the timeouts set via the flags, // since we don't have a config with timeout values we can use timeout := int(e.FetchTimeout.Seconds()) @@ -194,7 +218,12 @@ func (e *Engine) acquireConfig() (cfg types.Config, err error) { // (Re)Fetch the config if the cache is unreadable. cfg, err = e.fetchProviderConfig() - if err != nil { + if err == errors.ErrEmpty { + // Continue if the provider config was empty as we want to write an empty + // cache config for use by other stages. + cfg = emptyConfig + e.Logger.Info("%v: provider config was empty, continuing with empty cache config", err) + } else if err != nil { e.Logger.Warning("failed to fetch config: %s", err) return } @@ -222,7 +251,7 @@ func (e *Engine) acquireConfig() (cfg types.Config, err error) { } // Populate the config cache. - b, err = json.Marshal(cfg) + b, err := json.Marshal(cfg) if err != nil { e.Logger.Crit("failed to marshal cached config: %v", err) return diff --git a/tests/blackbox_test.go b/tests/blackbox_test.go index 189453e2b..b7c240feb 100644 --- a/tests/blackbox_test.go +++ b/tests/blackbox_test.go @@ -279,6 +279,10 @@ func outer(t *testing.T, test types.Test, negativeTests bool) error { appendEnv = append(appendEnv, "IGNITION_SYSTEM_CONFIG_DIR="+systemConfigDir) if !negativeTests { + if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv); err != nil { + return err + } + if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv); err != nil { return err } @@ -315,6 +319,10 @@ func outer(t *testing.T, test types.Test, negativeTests bool) error { } return nil } else { + if err := runIgnition(t, ctx, "fetch", "", tmpDirectory, appendEnv); err != nil { + return nil // error is expected + } + if err := runIgnition(t, ctx, "disks", "", tmpDirectory, appendEnv); err != nil { return nil // error is expected } diff --git a/tests/filesystem.go b/tests/filesystem.go index f6d711ca9..dcf0f2fc5 100644 --- a/tests/filesystem.go +++ b/tests/filesystem.go @@ -152,7 +152,7 @@ func umountPartition(p *types.Partition) error { // returns true if no error, false if error func runIgnition(t *testing.T, ctx context.Context, stage, root, cwd string, appendEnv []string) error { - args := []string{"-clear-cache", "-platform", "file", "-stage", stage, + args := []string{"-platform", "file", "-stage", stage, "-root", root, "-log-to-stdout", "--config-cache", filepath.Join(cwd, "ignition.json")} cmd := exec.CommandContext(ctx, "ignition", args...) t.Log("ignition", args) From bc770a00954ebc68a8e417b1e3c0a4e49ac6dfa5 Mon Sep 17 00:00:00 2001 From: Stephen Lowrie Date: Wed, 1 Jul 2020 21:20:54 -0500 Subject: [PATCH 2/2] internal/resource/http: clear CA compression on rewrite CAs are re-written into the fetched config as a base64 string to allow cached reads to not require an additional fetch. When compression support was added during the resource consolidation refactor for the 3.1.0 spec this interaction was missed. The blackbox tests did not catch this as they were always clearing the config cache on each run. --- internal/resource/http.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/resource/http.go b/internal/resource/http.go index 1cc621fc0..c3fec002b 100644 --- a/internal/resource/http.go +++ b/internal/resource/http.go @@ -209,6 +209,8 @@ func (f *Fetcher) RewriteCAsWithDataUrls(cas []types.Resource) error { // Clean HTTP headers cas[i].HTTPHeaders = nil + // the rewrite wipes the compression + cas[i].Compression = nil encoded := dataurl.EncodeBytes(blob) cas[i].Source = &encoded