From 92b54c490704429be97d30baf4a9e69cb2273741 Mon Sep 17 00:00:00 2001
From: LaMont Jones <lamontjones@google.com>
Date: Fri, 12 Jul 2024 12:26:26 -0700
Subject: [PATCH] Add more guardrails to release config compilation

Throw an error if a release config dir appears to contribute flagging
overrides to a release config without actually doing so (by having
release_configs/{name}.textproto).

Bug: None
Test: manual, TH
Change-Id: Ie71cd6898bda4b8da8d58c3e23fb89ed17ebebd1
---
 .../release_config_lib/release_configs.go     | 40 +++++++++++++++++++
 cmd/release_config/release_config_lib/util.go |  9 ++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/cmd/release_config/release_config_lib/release_configs.go b/cmd/release_config/release_config_lib/release_configs.go
index f2e138801..a4c884ce3 100644
--- a/cmd/release_config/release_config_lib/release_configs.go
+++ b/cmd/release_config/release_config_lib/release_configs.go
@@ -42,6 +42,10 @@ type ReleaseConfigMap struct {
 
 	// Flags declared this directory's flag_declarations/*.textproto
 	FlagDeclarations []rc_proto.FlagDeclaration
+
+	// Potential aconfig and build flag contributions in this map directory.
+	// This is used to detect errors.
+	FlagValueDirs map[string][]string
 }
 
 type ReleaseConfigDirMap map[string]int
@@ -317,6 +321,21 @@ func (configs *ReleaseConfigs) LoadReleaseConfigMap(path string, ConfigDirIndex
 		return err
 	}
 
+	subDirs := func(subdir string) (ret []string) {
+		if flagVersions, err := os.ReadDir(filepath.Join(dir, subdir)); err == nil {
+			for _, e := range flagVersions {
+				if e.IsDir() && validReleaseConfigName(e.Name()) {
+					ret = append(ret, e.Name())
+				}
+			}
+		}
+		return
+	}
+	m.FlagValueDirs = map[string][]string{
+		"aconfig":     subDirs("aconfig"),
+		"flag_values": subDirs("flag_values"),
+	}
+
 	err = WalkTextprotoFiles(dir, "release_configs", func(path string, d fs.DirEntry, err error) error {
 		releaseConfigContribution := &ReleaseConfigContribution{path: path, DeclarationIndex: ConfigDirIndex}
 		LoadMessage(path, &releaseConfigContribution.proto)
@@ -424,6 +443,27 @@ func (configs *ReleaseConfigs) GenerateReleaseConfigs(targetRelease string) erro
 		}
 	}
 
+	// Look for ignored flagging values.  Gather the entire list to make it easier to fix them.
+	errors := []string{}
+	for _, contrib := range configs.ReleaseConfigMaps {
+		dirName := filepath.Dir(contrib.path)
+		for k, names := range contrib.FlagValueDirs {
+			for _, rcName := range names {
+				if config, err := configs.GetReleaseConfig(rcName); err == nil {
+					rcPath := filepath.Join(dirName, "release_configs", fmt.Sprintf("%s.textproto", config.Name))
+					if _, err := os.Stat(rcPath); err != nil {
+						errors = append(errors, fmt.Sprintf("%s exists but %s does not contribute to %s",
+							filepath.Join(dirName, k, rcName), dirName, config.Name))
+					}
+				}
+
+			}
+		}
+	}
+	if len(errors) > 0 {
+		return fmt.Errorf("%s", strings.Join(errors, "\n"))
+	}
+
 	releaseConfig, err := configs.GetReleaseConfig(targetRelease)
 	if err != nil {
 		return err
diff --git a/cmd/release_config/release_config_lib/util.go b/cmd/release_config/release_config_lib/util.go
index 9919c7081..b149293c2 100644
--- a/cmd/release_config/release_config_lib/util.go
+++ b/cmd/release_config/release_config_lib/util.go
@@ -31,8 +31,9 @@ import (
 )
 
 var (
-	disableWarnings    bool
-	containerRegexp, _ = regexp.Compile("^[a-z][a-z0-9]*([._][a-z][a-z0-9]*)*$")
+	disableWarnings        bool
+	containerRegexp, _     = regexp.Compile("^[a-z][a-z0-9]*([._][a-z][a-z0-9]*)*$")
+	releaseConfigRegexp, _ = regexp.Compile("^[a-z][a-z0-9]*([._][a-z0-9]*)*$")
 )
 
 type StringList []string
@@ -179,6 +180,10 @@ func validContainer(container string) bool {
 	return containerRegexp.MatchString(container)
 }
 
+func validReleaseConfigName(name string) bool {
+	return releaseConfigRegexp.MatchString(name)
+}
+
 // Returns the default value for release config artifacts.
 func GetDefaultOutDir() string {
 	outEnv := os.Getenv("OUT_DIR")