internal/config: fix CR issues

.golangci.yaml

@@ -34,5 +34,11 @@ issues:
     - linters:
         - revive
       text: 'unused-parameter: parameter ''.*'' seems to be unused'
+    - linters:
+        - revive
+      text: 'exported: type name will be used as .* by other packages, and that stutters; consider calling this .*'
+    - linters:
+        - revive
+      text: 'redefines-builtin-id: redefinition of the built-in function .*'
 run:
   timeout: 5m

cmd/lava/internal/run/testdata/lava-run-test/go.mod

@@ -13,18 +13,18 @@ require (
 	dario.cat/mergo v1.0.0 // indirect
 	github.com/BurntSushi/toml v1.2.1 // indirect
 	github.com/Microsoft/go-winio v0.6.1 // indirect
-	github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371 // indirect
+	github.com/ProtonMail/go-crypto v1.1.5 // indirect
 	github.com/adevinta/vulcan-types v1.2.11 // indirect
 	github.com/aws/aws-sdk-go v1.51.0 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect
-	github.com/cyphar/filepath-securejoin v0.2.4 // indirect
+	github.com/cyphar/filepath-securejoin v0.3.6 // indirect
 	github.com/distribution/reference v0.5.0 // indirect
 	github.com/docker/distribution v2.8.3+incompatible // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
-	github.com/go-git/go-billy/v5 v5.5.0 // indirect
-	github.com/go-git/go-git/v5 v5.11.0 // indirect
+	github.com/go-git/go-billy/v5 v5.6.2 // indirect
+	github.com/go-git/go-git/v5 v5.13.2 // indirect
 	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -39,11 +39,11 @@ require (
 	github.com/kr/text v0.2.0 // indirect
 	github.com/miekg/dns v1.1.58 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
-	github.com/pjbgf/sha1cd v0.3.0 // indirect
-	github.com/rogpeppe/go-internal v1.11.0 // indirect
-	github.com/sergi/go-diff v1.3.1 // indirect
+	github.com/pjbgf/sha1cd v0.3.2 // indirect
+	github.com/rogpeppe/go-internal v1.12.0 // indirect
+	github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 // indirect
 	github.com/sirupsen/logrus v1.9.0 // indirect
-	github.com/skeema/knownhosts v1.2.1 // indirect
+	github.com/skeema/knownhosts v1.3.0 // indirect
 	github.com/xanzy/ssh-agent v0.3.3 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect

cmd/lava/internal/scan/scan.go

@@ -91,7 +91,7 @@ func scan(args []string) (int, error) {
 	startTime := time.Now()
 	metrics.Collect("start_time", startTime)
 
-	cfg, err := config.ParseFile(scanC)
+	cfg, err := config.Parse(scanC)
 	if err != nil {
 		return 0, fmt.Errorf("parse config file: %w", err)
 	}

internal/config/config.go

@@ -6,7 +6,6 @@ package config
 import (
 	"errors"
 	"fmt"
-	"io"
 	"log/slog"
 	"os"
 	"regexp"
@@ -85,30 +84,21 @@ type Config struct {
 // reEnv is used to replace embedded environment variables.
 var reEnv = regexp.MustCompile(`\$\{[a-zA-Z_][a-zA-Z_0-9]*\}`)
 
-// Parse returns a parsed Lava configuration given an [io.Reader].
-func Parse(r io.Reader, path string) (Config, error) {
-	b, err := io.ReadAll(r)
-	if err != nil {
-		return Config{}, fmt.Errorf("read config: %w", err)
-	}
-
-	cfg, err := Decode(b)
-	if err != nil {
-		return Config{}, fmt.Errorf("decode config: %w", err)
-	}
-	dag, err := NewDAG(cfg, path)
+// Parse returns a parsed Lava configuration given a path to a
+// file.
+func Parse(path string) (Config, error) {
+	dag, err := NewConfigGraph(path)
 	if err != nil {
 		return Config{}, fmt.Errorf("build dag: %w", err)
 	}
 	if err = dag.Resolve(); err != nil {
 		return Config{}, fmt.Errorf("resolve dag: %w", err)
 	}
 
-	if err = cfg.validate(); err != nil {
+	if err = dag.configs[path].validate(); err != nil {
 		return Config{}, fmt.Errorf("validate config: %w", err)
 	}
-
-	return cfg, nil
+	return dag.configs[path], nil
 }
 
 // Decode decodes from a slice of bytes to a [Config] structure.
@@ -130,17 +120,6 @@ func Decode(b []byte) (Config, error) {
 	return cfg, nil
 }
 
-// ParseFile returns a parsed Lava configuration given a path to a
-// file.
-func ParseFile(path string) (Config, error) {
-	f, err := os.Open(path)
-	if err != nil {
-		return Config{}, fmt.Errorf("open config file: %w", err)
-	}
-	defer f.Close()
-	return Parse(f, path)
-}
-
 // validate validates the Lava configuration.
 func (c Config) validate() error {
 	// Lava version validation.

internal/config/config_test.go

@@ -14,8 +14,6 @@ import (
 	agentconfig "github.com/adevinta/vulcan-agent/config"
 	types "github.com/adevinta/vulcan-types"
 	"github.com/google/go-cmp/cmp"
-
-	"github.com/adevinta/lava/internal/urlutil"
 )
 
 func TestParseFile(t *testing.T) {
@@ -276,7 +274,7 @@ func TestParseFile(t *testing.T) {
 			for k, v := range tt.envs {
 				t.Setenv(k, v)
 			}
-			got, err := ParseFile(tt.file)
+			got, err := Parse(tt.file)
 
 			switch {
 			case tt.wantErr != nil:
@@ -465,85 +463,3 @@ func mustParseExpDate(date string) ExpirationDate {
 	}
 	return ExpirationDate{Time: t}
 }
-
-func TestParse_FromReader(t *testing.T) {
-	tests := []struct {
-		name          string
-		file          string
-		envs          map[string]string
-		want          Config
-		wantErr       error
-		wantErrRegexp *regexp.Regexp
-	}{
-		{
-			name: "valid",
-			file: "testdata/valid.yaml",
-			want: Config{
-				LavaVersion: ptr("v1.0.0"),
-				ChecktypeURLs: []string{
-					"checktypes.json",
-				},
-				Targets: []Target{
-					{
-						Identifier: "example.com",
-						AssetType:  types.DomainName,
-					},
-				},
-			},
-		},
-		{
-			name: "valid env",
-			file: "testdata/valid_env.yaml",
-			want: Config{
-				LavaVersion: ptr("v1.0.0"),
-				ChecktypeURLs: []string{
-					"checktypes.json",
-				},
-				Targets: []Target{
-					{
-						Identifier: "example.com",
-						AssetType:  types.DomainName,
-					},
-				},
-			},
-			envs: map[string]string{
-				"TARGET":           "example.com",
-				"CHECK_types_URL1": "checktypes.json",
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			for k, v := range tt.envs {
-				t.Setenv(k, v)
-			}
-
-			data, err := urlutil.Get(tt.file)
-			if err != nil {
-				t.Errorf("could not resolve %s: %v", tt.file, err)
-			}
-			r := bytes.NewReader(data)
-			got, err := Parse(r, "")
-
-			switch {
-			case tt.wantErr != nil:
-				if !errors.Is(err, tt.wantErr) {
-					t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr)
-				}
-			case tt.wantErrRegexp != nil:
-				if err == nil {
-					t.Errorf("unexpected nil error: want: %v", tt.wantErrRegexp)
-				} else if !tt.wantErrRegexp.MatchString(err.Error()) {
-					t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErrRegexp)
-				}
-			default:
-				if err != nil {
-					t.Errorf("unexpected error: %v", err)
-				}
-			}
-			if diff := cmp.Diff(tt.want, got); diff != "" {
-				t.Errorf("configs mismatch (-want +got):\n%v", diff)
-			}
-		})
-	}
-}

internal/config/resolver.go → internal/config/configgraph.go

@@ -6,55 +6,44 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"log/slog"
 
 	"github.com/adevinta/lava/internal/config/dag"
 	"github.com/adevinta/lava/internal/urlutil"
 )
 
-// DAG is the representation of the structure of includes.
-type DAG struct {
+// ConfigGraph is the representation of the structure of includes.
+type ConfigGraph struct {
 	dag     *dag.DAG
 	configs map[string]Config
 }
 
-// NewDAG creates a new DAG that represents the whole configuration.
-func NewDAG(cfg Config, url string) (DAG, error) {
-	d := dag.NewDAG()
-	configs := map[string]Config{
-		url: cfg,
+// NewConfigGraph creates a new ConfigGraph that represents the whole configuration.
+func NewConfigGraph(url string) (ConfigGraph, error) {
+	d := dag.New()
+	configs := map[string]Config{}
+	if err := discoverConfig(url, "", d, configs); err != nil {
+		return ConfigGraph{}, fmt.Errorf("could not discover config %s: %w", url, err)
 	}
-	if _, err := d.AddVertex(url); err != nil {
-		return DAG{}, fmt.Errorf("could not add vertex: %w", err)
-	}
-
-	unique := make(map[string]struct{})
-	for _, incl := range cfg.Includes {
-		// Skip duplicates.
-		if _, ok := unique[incl]; ok {
-			continue
-		}
-		unique[incl] = struct{}{}
-		if err := discoverConfig(incl, url, d, configs); err != nil {
-			return DAG{}, fmt.Errorf("could not discover config %s: %w", incl, err)
-		}
-	}
-
-	return DAG{
+	return ConfigGraph{
 		dag:     d,
 		configs: configs,
 	}, nil
 }
 
 // discoverConfig explores the included configs to build the dag.
 func discoverConfig(url, parent string, d *dag.DAG, configs map[string]Config) error {
-	if !d.HasVertexBeenAdded(url) {
+	if !d.Contains(url) {
 		_, err := d.AddVertex(url)
 		if err != nil {
 			return fmt.Errorf("could not add vertex: %w", err)
 		}
 	}
-	if err := d.AddEdge(parent, url); err != nil {
-		return fmt.Errorf("could not add edge: %w", err)
+	// We don't add edges for the root url.
+	if parent != "" {
+		if err := d.AddEdge(parent, url); err != nil {
+			return fmt.Errorf("could not add edge: %w", err)
+		}
 	}
 	// Retrieve the parsed config of the include.
 	data, err := urlutil.Get(url)
@@ -76,6 +65,7 @@ func discoverConfig(url, parent string, d *dag.DAG, configs map[string]Config) e
 	for _, incl := range cfg.Includes {
 		// Skip duplicates.
 		if _, ok := unique[incl]; ok {
+			slog.Warn("duplicate include config found for the url", "include", incl)
 			continue
 		}
 		unique[incl] = struct{}{}
@@ -87,7 +77,7 @@ func discoverConfig(url, parent string, d *dag.DAG, configs map[string]Config) e
 }
 
 // Resolve walks the dag and merge the configuration.
-func (d *DAG) Resolve() error {
+func (d *ConfigGraph) Resolve() error {
 	// TODO: Walk the dag and merge configurations.
 	return nil
 }