From 699aa6203c445e63df260134e7c94ec0bfa2f322 Mon Sep 17 00:00:00 2001 From: Itxaka Date: Fri, 15 Sep 2023 20:59:31 +0200 Subject: [PATCH] Read the first ten lines of a config to check for headers (#51) There could be comments before the cloud-config header or jinja expressions as we found already[0] so our current header check its a bit strict. This patch makes it so er read the first ten lines of the config source, check for # at the start and then check for the headers as we did before, so we are a bit more letinent in case the cloud-config header is not the first thing in the file Signed-off-by: Itxaka --- collector/collector.go | 26 +++++++++++++----- collector/collector_test.go | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/collector/collector.go b/collector/collector.go index 28858b51..1d0ac494 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -400,14 +400,26 @@ func fetchRemoteConfig(url string) (*Config, error) { } func HasValidHeader(data string) bool { - header := strings.SplitN(data, "\n", 2)[0] - - // Trim trailing whitespaces - header = strings.TrimRightFunc(header, unicode.IsSpace) + // Get the first 10 lines + headers := strings.SplitN(data, "\n", 10) + + // iterate over them as there could be comments or the jinja template info: + // https://cloudinit.readthedocs.io/en/latest/explanation/instancedata.html#example-cloud-config-with-instance-data + + for _, line := range headers { + // Trim trailing whitespaces + header := strings.TrimRightFunc(line, unicode.IsSpace) + // If it starts with a hash check it, in case its a huge line, we dont want to waste time + if strings.HasPrefix(header, "#") { + // NOTE: we also allow "legacy" headers. Should only allow #cloud-config at + // some point. + if (header == DefaultHeader) || (header == "#kairos-config") || (header == "#node-config") { + return true + } + } + } - // NOTE: we also allow "legacy" headers. Should only allow #cloud-config at - // some point. - return (header == DefaultHeader) || (header == "#kairos-config") || (header == "#node-config") + return false } func (c Config) Query(s string) (res string, err error) { diff --git a/collector/collector_test.go b/collector/collector_test.go index cfc3d961..246b6445 100644 --- a/collector/collector_test.go +++ b/collector/collector_test.go @@ -833,6 +833,60 @@ remote_key_2: remote_value_2`), os.ModePerm) Expect(v).To(Equal("remote_value_1")) }) }) + Context("when files have comments before the headers or jinja declarations", func() { + var tmpDir string + var err error + + BeforeEach(func() { + tmpDir, err = os.MkdirTemp("", "config") + Expect(err).ToNot(HaveOccurred()) + + // Local configs + err = os.WriteFile(path.Join(tmpDir, "local_config.yaml"), []byte(`## template: jinja +#cloud-config +local_key_1: local_value_1 +`), os.ModePerm) + Expect(err).ToNot(HaveOccurred()) + + // comments before the header + err = os.WriteFile(path.Join(tmpDir, "local_config_2.yaml"), + []byte(` +# this is a comment +## then another comment +#and the last one + +#cloud-config +local_key_2: local_value_2 +`), os.ModePerm) + Expect(err).ToNot(HaveOccurred()) + + }) + + AfterEach(func() { + err = os.RemoveAll(tmpDir) + Expect(err).ToNot(HaveOccurred()) + }) + + It("reads them", func() { + o := &Options{} + err := o.Apply(Directories(tmpDir), NoLogs) + Expect(err).ToNot(HaveOccurred()) + + c, err := Scan(o, FilterKeysTest) + Expect(err).ToNot(HaveOccurred()) + + Expect((*c)["local_key_1"]).ToNot(BeNil()) + Expect((*c)["local_key_2"]).ToNot(BeNil()) + + v, ok := (*c)["local_key_1"].(string) + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("local_value_1")) + + v, ok = (*c)["local_key_2"].(string) + Expect(ok).To(BeTrue()) + Expect(v).To(Equal("local_value_2")) + }) + }) }) Describe("String", func() {