Merge pull request GoogleCloudPlatform#2243 from GoogleCloudPlatform/…

…release-candidate

Release v1.28.1

.github/workflows/dependency-review.yml

@@ -26,6 +26,7 @@ permissions:
 
 jobs:
   dependency-review:
+    if: github.repository == 'GoogleCloudPlatform/hpc-toolkit'
     runs-on: ubuntu-latest
     steps:
     - name: 'Checkout Repository'

.github/workflows/pr-label-validation.yml

@@ -32,6 +32,7 @@ on:
 
 jobs:
   pr-label-validation:
+    if: github.repository == 'GoogleCloudPlatform/hpc-toolkit'
     runs-on: ubuntu-latest
     permissions:
       pull-requests: read

.github/workflows/pr-precommit.yml

@@ -0,0 +1,55 @@
+# Copyright 2024 "Google LLC"
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+---
+name: 'Use pre-commit to validate Pull Request'
+
+# yamllint disable-line rule:truthy
+on:
+  pull_request:
+    types:
+    - edited
+    - opened
+    - labeled
+    - synchronize
+    branches:
+    - develop
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-python@v5
+      with:
+        python-version: '3.10'
+        check-latest: true
+        cache: 'pip'
+    - uses: actions/setup-go@v5
+      with:
+        go-version: '1.22'
+        check-latest: true
+    - run: make install-dev-deps
+    - uses: terraform-linters/setup-tflint@v4
+      with:
+        tflint_version: v0.49.0
+    - uses: pre-commit/[email protected]
+    - uses: pre-commit-ci/[email protected]
+      # this if statement looks funny but it ensures that this step runs
+      # only if: user has applied "pre-commit-autofix" label
+      # even if: job has failed
+      # not if: job is canceled
+      if: |
+        (success() || failure()) &&
+        contains(github.event.pull_request.labels.*.name, 'pre-commit-autofix')

.pre-commit-config.yaml

@@ -15,7 +15,7 @@
 ---
 repos:
 - repo: https://github.com/antonbabenko/pre-commit-terraform
-  rev: v1.82.0
+  rev: v1.86.0
   hooks:
   - id: terraform_fmt
   - id: terraform_tflint
@@ -82,16 +82,16 @@ repos:
 #   hooks:
 #   - id: go-critic
 - repo: https://github.com/Bahjat/pre-commit-golang
-  rev: v1.0.2
+  rev: v1.0.3
   hooks:
   - id: go-static-check
 - repo: https://github.com/adrienverge/yamllint
-  rev: v1.32.0
+  rev: v1.34.0
   hooks:
   - id: yamllint
-    args: [-c=.yamllint]
+    args: [-c=.yamllint, --no-warnings]
 - repo: https://github.com/jackdewinter/pymarkdown
-  rev: v0.9.12
+  rev: v0.9.17
   hooks:
   - id: pymarkdown
     # Rules at https://github.com/jackdewinter/pymarkdown/tree/main/docs/rules
@@ -101,7 +101,8 @@ repos:
     # MD034 - Bare URL used
     # MD041 - First line in file should be a top level header
     # MD046 - Code block style
-    args: [--disable-rules, "MD013,MD022,MD033,MD034,MD041,MD046", scan]
+    # MD024 - Multiple headings cannot contain the same content.
+    args: [--disable-rules, "MD013,MD022,MD033,MD034,MD041,MD046,MD024", scan]
 - repo: https://github.com/jumanjihouse/pre-commit-hooks
   rev: "3.0.0"
   hooks:
@@ -110,7 +111,7 @@ repos:
   - id: shfmt
     exclude: ".*tpl"
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.4.0
+  rev: v4.5.0
   hooks:
   - id: end-of-file-fixer
 - repo: https://github.com/codespell-project/codespell

Makefile

@@ -20,7 +20,7 @@ ifneq (, $(shell which git))
 ifneq (,$(wildcard .git))
 ## GIT DIRECTORY EXISTS
 GIT_TAG_VERSION=$(shell git tag --points-at HEAD)
-GIT_BRANCH=$(shell git branch --show-current)
+GIT_BRANCH=$(shell $(SHELL) -c 'git branch --show-current || git rev-parse --abbrev-ref HEAD' 2>/dev/null)
 GIT_COMMIT_INFO=$(shell git describe --tags --dirty --long --always)
 GIT_COMMIT_HASH=$(shell git rev-parse HEAD)
 GIT_INITIAL_HASH=$(shell git rev-list --max-parents=0 HEAD)

cmd/color.go

@@ -30,7 +30,7 @@ func init() {
 }
 
 func addColorFlag(flagset *pflag.FlagSet) {
-	flagset.BoolVar(&noColorFlag, "no-color", true, "Disable colorized output.")
+	flagset.BoolVar(&noColorFlag, "no-color", false, "Disable colorized output.")
 }
 
 func initColor() {

cmd/create.go

@@ -52,6 +52,10 @@ func init() {
 			"Note: Terraform state IS preserved. \n"+
 			"Note: Terraform workspaces are NOT supported (behavior undefined). \n"+
 			"Note: Packer is NOT supported.")
+	createCmd.Flags().BoolVar(&forceOverwrite, "force", false,
+		"Forces overwrite of existing deployment directory. \n"+
+			"If set, --overwrite-deployment is implied. \n"+
+			"No validation is performed on the existing deployment directory.")
 	rootCmd.AddCommand(createCmd)
 }
 
@@ -62,6 +66,7 @@ var (
 
 	cliBEConfigVars     []string
 	overwriteDeployment bool
+	forceOverwrite      bool
 	validationLevel     string
 	validationLevelDesc = "Set validation level to one of (\"ERROR\", \"WARNING\", \"IGNORE\")"
 	validatorsToSkip    []string
@@ -80,7 +85,7 @@ var (
 func runCreateCmd(cmd *cobra.Command, args []string) {
 	dc := expandOrDie(args[0])
 	deplDir := filepath.Join(outputDir, dc.Config.DeploymentName())
-	checkErr(checkOverwriteAllowed(deplDir, dc.Config, overwriteDeployment))
+	checkErr(checkOverwriteAllowed(deplDir, dc.Config, overwriteDeployment, forceOverwrite))
 	checkErr(modulewriter.WriteDeployment(dc, deplDir))
 
 	logging.Info("To deploy your infrastructure please run:")
@@ -110,7 +115,7 @@ func expandOrDie(path string) config.DeploymentConfig {
 		logging.Fatal("Failed to set the backend config at CLI: %v", err)
 	}
 	checkErr(setValidationLevel(&dc.Config, validationLevel))
-	checkErr(skipValidators(&dc))
+	skipValidators(&dc)
 
 	if dc.Config.GhpcVersion != "" {
 		logging.Info("ghpc_version setting is ignored.")
@@ -159,58 +164,6 @@ func validateMaybeDie(bp config.Blueprint, ctx config.YamlCtx) {
 
 }
 
-func findPos(path config.Path, ctx config.YamlCtx) (config.Pos, bool) {
-	pos, ok := ctx.Pos(path)
-	for !ok && path.Parent() != nil {
-		path = path.Parent()
-		pos, ok = ctx.Pos(path)
-	}
-	return pos, ok
-}
-
-func renderError(err error, ctx config.YamlCtx) string {
-	switch te := err.(type) {
-	case config.Errors:
-		var sb strings.Builder
-		for _, e := range te.Errors {
-			sb.WriteString(renderError(e, ctx))
-			sb.WriteString("\n")
-		}
-		return sb.String()
-	case validators.ValidatorError:
-		title := boldRed(fmt.Sprintf("validator %q failed:", te.Validator))
-		return fmt.Sprintf("%s\n%v\n", title, renderError(te.Err, ctx))
-	case config.BpError:
-		if pos, ok := findPos(te.Path, ctx); ok {
-			return renderRichError(te.Err, pos, ctx)
-		}
-		return renderError(te.Err, ctx)
-	default:
-		return err.Error()
-	}
-}
-
-func renderRichError(err error, pos config.Pos, ctx config.YamlCtx) string {
-	line := pos.Line - 1
-	if line < 0 {
-		line = 0
-	}
-	if line >= len(ctx.Lines) {
-		line = len(ctx.Lines) - 1
-	}
-
-	pref := fmt.Sprintf("%d: ", pos.Line)
-	arrow := " "
-	if pos.Column > 0 {
-		spaces := strings.Repeat(" ", len(pref)+pos.Column-1)
-		arrow = spaces + "^"
-	}
-
-	return fmt.Sprintf(`%s: %s
-%s%s
-%s`, boldRed("Error"), err, pref, ctx.Lines[line], arrow)
-}
-
 func setCLIVariables(bp *config.Blueprint, s []string) error {
 	for _, cliVar := range s {
 		arr := strings.SplitN(cliVar, "=", 2)
@@ -268,16 +221,10 @@ func setValidationLevel(bp *config.Blueprint, s string) error {
 	return nil
 }
 
-func skipValidators(dc *config.DeploymentConfig) error {
-	if validatorsToSkip == nil {
-		return nil
-	}
+func skipValidators(dc *config.DeploymentConfig) {
 	for _, v := range validatorsToSkip {
-		if err := dc.SkipValidator(v); err != nil {
-			return err
-		}
+		dc.SkipValidator(v)
 	}
-	return nil
 }
 
 func filterYaml(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
@@ -287,29 +234,37 @@ func filterYaml(cmd *cobra.Command, args []string, toComplete string) ([]string,
 	return []string{"yaml", "yml"}, cobra.ShellCompDirectiveFilterFileExt
 }
 
+func forceErr(err error) error {
+	return config.HintError{
+		Err:  err,
+		Hint: "Use `--force` to overwrite the deployment anyway. Proceed at your own risk."}
+}
+
 // Determines if overwrite is allowed
-func checkOverwriteAllowed(depDir string, bp config.Blueprint, overwriteFlag bool) error {
-	if _, err := os.Stat(depDir); os.IsNotExist(err) {
+func checkOverwriteAllowed(depDir string, bp config.Blueprint, overwriteFlag bool, forceFlag bool) error {
+	if _, err := os.Stat(depDir); os.IsNotExist(err) || forceFlag {
 		return nil // all good, no previous deployment
 	}
 
 	if _, err := os.Stat(modulewriter.HiddenGhpcDir(depDir)); os.IsNotExist(err) {
 		// hidden ghpc dir does not exist
-		return fmt.Errorf("folder %q already exists, and it is not a valid GHPC deployment folder", depDir)
+		return forceErr(fmt.Errorf("folder %q already exists, and it is not a valid GHPC deployment folder", depDir))
 	}
 
 	// try to get previous deployment
 	expPath := filepath.Join(modulewriter.ArtifactsDir(depDir), modulewriter.ExpandedBlueprintName)
 	if _, err := os.Stat(expPath); os.IsNotExist(err) {
-		return fmt.Errorf("expanded blueprint file %q is missing, this could be a result of changing GHPC version between consecutive deployments", expPath)
+		return forceErr(fmt.Errorf("expanded blueprint file %q is missing, this could be a result of changing GHPC version between consecutive deployments", expPath))
 	}
 	prev, _, err := config.NewDeploymentConfig(expPath)
 	if err != nil {
-		return err
+		return forceErr(err)
 	}
 
 	if prev.Config.GhpcVersion != bp.GhpcVersion {
-		logging.Info("WARNING: ghpc_version has changed from %q to %q, using different versions of GHPC to update a live deployment is not officially supported. Proceed at your own risk", prev.Config.GhpcVersion, bp.GhpcVersion)
+		return forceErr(fmt.Errorf(
+			"ghpc_version has changed from %q to %q, using different versions of GHPC to update a live deployment is not officially supported",
+			prev.Config.GhpcVersion, bp.GhpcVersion))
 	}
 
 	if !overwriteFlag {
@@ -323,7 +278,7 @@ func checkOverwriteAllowed(depDir string, bp config.Blueprint, overwriteFlag boo
 
 	for _, g := range prev.Config.DeploymentGroups {
 		if !newGroups[g.Name] {
-			return fmt.Errorf("you are attempting to remove a deployment group %q, which is not supported", g.Name)
+			return forceErr(fmt.Errorf("you are attempting to remove a deployment group %q, which is not supported", g.Name))
 		}
 	}