MustParseReference only accepts consts (#964)

hack/presubmit.sh

@@ -47,3 +47,5 @@ pushd ${PROJECT_ROOT}/pkg/authn/k8schain
 trap popd EXIT
 
 go test ./...
+
+./pkg/name/internal/must_test.sh

pkg/name/internal/must_test.go

@@ -0,0 +1,26 @@
+// +build compile
+
+// Copyright 2021 Google LLC All Rights Reserved.
+//
+// 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.
+
+package internal
+
+import (
+	"strings"
+
+	"github.com/google/go-containerregistry/pkg/name"
+)
+
+// This shouldn't compile.
+var _ = name.MustParseReference(strings.Join([]string{"valid", "string"}, "/"))

pkg/name/internal/must_test.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+
+# Copyright 2021 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.
+
+set -o nounset
+set -o pipefail
+
+# Trying to compile without the build tag should work.
+go test ./pkg/name/internal
+
+# Actually trying to compile should fail.
+go test -tags=compile ./pkg/name/internal
+if [[ $? -eq 0 ]]; then
+  echo "pkg/name/internal test compiled successfully, expected failure"
+  exit 1
+fi
+echo "pkg/name/internal test successfully did not compile"

pkg/name/ref.go

@@ -48,9 +48,27 @@ func ParseReference(s string, opts ...Option) (Reference, error) {
 
 }
 
-// MustParseReference behaves like ParseReference, but panics instead of returning an error.
-func MustParseReference(s string, opts ...Option) Reference {
-	ref, err := ParseReference(s, opts...)
+type stringConst string
+
+// MustParseReference behaves like ParseReference, but panics instead of
+// returning an error. It's intended for use in tests, or when a value is
+// expected to be valid at code authoring time.
+//
+// To discourage its use in scenarios where the value is not known at code
+// authoring time, it must be passed a string constant:
+//
+//    const str = "valid/string"
+//    MustParseReference(str)
+//    MustParseReference("another/valid/string")
+//    MustParseReference(str + "/and/more")
+//
+// These will not compile:
+//
+//    var str = "valid/string"
+//    MustParseReference(str)
+//    MustParseReference(strings.Join([]string{"valid", "string"}, "/"))
+func MustParseReference(s stringConst, opts ...Option) Reference {
+	ref, err := ParseReference(string(s), opts...)
 	if err != nil {
 		panic(err)
 	}

pkg/name/ref_test.go

@@ -126,7 +126,7 @@ func TestMustParseReference(t *testing.T) {
 					t.Errorf("MustParseReference(%q, WeakValidation); panic: %v", name, err)
 				}
 			}()
-			MustParseReference(name, WeakValidation)
+			MustParseReference(stringConst(name), WeakValidation)
 		}()
 	}
 
@@ -138,16 +138,23 @@ func TestMustParseReference(t *testing.T) {
 					t.Errorf("MustParseReference(%q, StrictValidation); panic: %v", name, err)
 				}
 			}()
-			MustParseReference(name, StrictValidation)
+			MustParseReference(stringConst(name), StrictValidation)
 		}()
 	}
 
 	badNames := append(badTagNames, badDigestNames...)
 	for _, name := range badNames {
 		func() {
 			defer func() { recover() }()
-			ref := MustParseReference(name, WeakValidation)
+			ref := MustParseReference(stringConst(name), WeakValidation)
 			t.Errorf("MustParseReference(%q, WeakValidation) should panic, got: %#v", name, ref)
 		}()
 	}
 }
+
+// Test that MustParseReference can accept a const string or string value.
+const str = "valid/string"
+
+var _ = MustParseReference(str)
+var _ = MustParseReference("valid/string")
+var _ = MustParseReference("valid/prefix/" + str)