Merge pull request #907 from lcarva/HACBS-2079

cmd/validate/image.go

@@ -159,18 +159,16 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command {
 
 			  ec validate image --image registry/name:tag --output data=<path>
 
-			Validate a single image with keyless workflow. This is an experimental feature
-			that requires setting the EC_EXPERIMENTAL environment variable to "1".
+			Validate a single image with keyless workflow.
 
-			  EC_EXPERIMENTAL="1" ec validate image --image registry/name:tag --policy my-policy \
+			  ec validate image --image registry/name:tag --policy my-policy \
 			    --certificate-identity 'https://github.com/user/repo/.github/workflows/push.yaml@refs/heads/main' \
 			    --certificate-oidc-issuer 'https://token.actions.githubusercontent.com' \
 			    --rekor-url 'https://rekor.sigstore.dev'
 
-			Use a regular expression to match certificate attributes. This is an experimental
-			feature that requires setting the EC_EXPERIMENTAL environment variable to "1".
+			Use a regular expression to match certificate attributes.
 
-			  EC_EXPERIMENTAL="1" ec validate image --image registry/name:tag --policy my-policy \
+			  ec validate image --image registry/name:tag --policy my-policy \
 			    --certificate-identity-regexp '^https://github\.com' \
 			    --certificate-oidc-issuer-regexp 'githubusercontent' \
 			    --rekor-url 'https://rekor.sigstore.dev'
@@ -372,16 +370,16 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command {
 		"Skip Rekor transparency log checks during validation.")
 
 	cmd.Flags().StringVar(&data.certificateIdentity, "certificate-identity", data.certificateIdentity,
-		"EXPERIMENTAL. URL of the certificate identity for keyless verification")
+		"URL of the certificate identity for keyless verification")
 
 	cmd.Flags().StringVar(&data.certificateIdentityRegExp, "certificate-identity-regexp", data.certificateIdentityRegExp,
-		"EXPERIMENTAL. Regular expression for the URL of the certificate identity for keyless verification")
+		"Regular expression for the URL of the certificate identity for keyless verification")
 
 	cmd.Flags().StringVar(&data.certificateOIDCIssuer, "certificate-oidc-issuer", data.certificateOIDCIssuer,
-		"EXPERIMENTAL. URL of the certificate OIDC issuer for keyless verification")
+		"URL of the certificate OIDC issuer for keyless verification")
 
 	cmd.Flags().StringVar(&data.certificateOIDCIssuerRegExp, "certificate-oidc-issuer-regexp", data.certificateOIDCIssuerRegExp,
-		"EXPERIMENTAL. Regular expresssion for the URL of the certificate OIDC issuer for keyless verification")
+		"Regular expresssion for the URL of the certificate OIDC issuer for keyless verification")
 
 	// Deprecated: images replaced this
 	cmd.Flags().StringVarP(&data.filePath, "file-path", "f", data.filePath,

cmd/validate/image_test.go

@@ -464,8 +464,6 @@ func Test_ValidateImageCommandKeyless(t *testing.T) {
 		"my-certificate-oidc-issuer-regexp",
 	})
 
-	t.Setenv("EC_EXPERIMENTAL", "1")
-
 	utils.SetTestRekorPublicKey(t)
 	utils.SetTestFulcioRoots(t)
 	utils.SetTestCTLogPublicKey(t)

features/validate_image.feature

@@ -95,7 +95,6 @@ Feature: evaluate enterprise contract
       ]
     }
     """
-    Given the environment variable is set "EC_EXPERIMENTAL=1"
     When ec command is run with "validate image --image ${REGISTRY}/acceptance/ec-happy-day-keyless --policy acceptance/ec-policy --certificate-oidc-issuer ${CERT_ISSUER} --certificate-identity ${CERT_IDENTITY} --strict --show-successes"
     Then the exit status should be 0
     Then the output should match the snapshot
@@ -142,7 +141,6 @@ Feature: evaluate enterprise contract
       ]
     }
     """
-    Given the environment variable is set "EC_EXPERIMENTAL=1"
     When ec command is run with "validate image --image ${REGISTRY}/acceptance/unexpected-keyless-cert --policy acceptance/ec-policy --certificate-oidc-issuer https://spam.cluster.local --certificate-identity https://kubernetes.io/namespaces/bacon/serviceaccounts/eggs --strict --show-successes"
     Then the exit status should be 1
     Then the output should match the snapshot

internal/output/output_test.go

@@ -838,7 +838,6 @@ func TestSetImageSignatureCheckFromError(t *testing.T) {
 		expectedPassed bool
 		expectedResult *evaluator.Result
 		policy         func(context.Context) policy.Policy
-		experimental   bool
 	}{
 		{
 			name:           "success",
@@ -890,18 +889,13 @@ func TestSetImageSignatureCheckFromError(t *testing.T) {
 				require.NoError(t, err)
 				return p
 			},
-			experimental: true,
 		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			ctx := context.Background()
 
-			if c.experimental {
-				t.Setenv("EC_EXPERIMENTAL", "1")
-			}
-
 			var p policy.Policy
 			if c.policy != nil {
 				utils.SetTestRekorPublicKey(t)
@@ -930,7 +924,6 @@ func TestSetAttestationSignatureCheckFromError(t *testing.T) {
 		expectedPassed bool
 		expectedResult *evaluator.Result
 		policy         func(context.Context) policy.Policy
-		experimental   bool
 	}{
 		{
 			name:           "success",
@@ -982,18 +975,13 @@ func TestSetAttestationSignatureCheckFromError(t *testing.T) {
 				require.NoError(t, err)
 				return p
 			},
-			experimental: true,
 		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			ctx := context.Background()
 
-			if c.experimental {
-				t.Setenv("EC_EXPERIMENTAL", "1")
-			}
-
 			var p policy.Policy
 			if c.policy != nil {
 				utils.SetTestRekorPublicKey(t)

internal/policy/policy.go

@@ -37,7 +37,6 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"github.com/enterprise-contract/ec-cli/internal/kubernetes"
-	"github.com/enterprise-contract/ec-cli/internal/utils"
 	e "github.com/enterprise-contract/ec-cli/pkg/error"
 )
 
@@ -77,12 +76,11 @@ type policy struct {
 
 // PublicKeyPEM returns the PublicKey in PEM format.
 func (p *policy) PublicKeyPEM() ([]byte, error) {
+	// Public key is not involved when using keyless verification
+	if p.Keyless() {
+		return []byte{}, nil
+	}
 	if p.checkOpts == nil || p.checkOpts.SigVerifier == nil {
-		if keylessEnabled() {
-			// When using the experimental keyless functionality, it is not expected
-			// that SigVerifier will be set.
-			return []byte{}, nil
-		}
 		return nil, errors.New("no check options or sig verifier configured")
 	}
 	pk, err := p.checkOpts.SigVerifier.PublicKey()
@@ -109,7 +107,7 @@ func (p *policy) Identity() cosign.Identity {
 
 // Keyless returns whether or not the Policy uses the keyless workflow for verification.
 func (p *policy) Keyless() bool {
-	return keylessEnabled() && p.PublicKey == ""
+	return p.PublicKey == ""
 }
 
 type Options struct {
@@ -189,10 +187,6 @@ func NewPolicy(ctx context.Context, opts Options) (Policy, error) {
 	}
 
 	if p.PublicKey == "" {
-		if !keylessEnabled() {
-			return nil, errors.New("policy must provide a public key")
-		}
-
 		if opts.Identity != (cosign.Identity{}) {
 			p.identity = opts.Identity
 		} else if p.EnterpriseContractPolicySpec.Identity != nil {
@@ -443,7 +437,3 @@ func validateIdentity(identity cosign.Identity) error {
 
 	return errs
 }
-
-func keylessEnabled() bool {
-	return utils.Experimental()
-}

internal/policy/policy_test.go

@@ -176,10 +176,6 @@ func TestNewPolicyFailures(t *testing.T) {
 		policyRef  string
 		k8sError   bool
 	}{
-		{
-			name:       "public key is required",
-			errorCause: "policy must provide a public key",
-		},
 		{
 			name:       "invalid inline JSON",
 			policyRef:  `{"invalid": "json""}`,
@@ -229,7 +225,6 @@ func TestCheckOpts(t *testing.T) {
 		ignoreRekor     bool
 		publicKey       string
 		remotePublicKey string
-		setExperimental bool
 		identity        cosign.Identity
 		expectKeyless   bool
 		err             string
@@ -259,97 +254,83 @@ func TestCheckOpts(t *testing.T) {
 			publicKey:   utils.TestPublicKey,
 		},
 		{
-			name: "missing public key",
-			err:  "policy must provide a public key",
-		},
-		{
-			name:            "keyless",
-			rekorUrl:        utils.TestRekorURL,
-			setExperimental: true,
-			expectKeyless:   true,
+			name:          "keyless",
+			rekorUrl:      utils.TestRekorURL,
+			expectKeyless: true,
 			identity: cosign.Identity{
 				Issuer:  "my-issuer",
 				Subject: "my-subject",
 			},
 		},
 		{
-			name:            "keyless without rekor",
-			ignoreRekor:     true,
-			setExperimental: true,
-			expectKeyless:   true,
+			name:          "keyless without rekor",
+			ignoreRekor:   true,
+			expectKeyless: true,
 			identity: cosign.Identity{
 				Issuer:  "my-issuer",
 				Subject: "my-subject",
 			},
 		},
 		{
-			name:            "keyless with regexp issuer",
-			rekorUrl:        utils.TestRekorURL,
-			setExperimental: true,
-			expectKeyless:   true,
+			name:          "keyless with regexp issuer",
+			rekorUrl:      utils.TestRekorURL,
+			expectKeyless: true,
 			identity: cosign.Identity{
 				IssuerRegExp: "my-issuer-regexp",
 				Subject:      "my-subject",
 			},
 		},
 		{
-			name:            "keyless with regexp subject",
-			rekorUrl:        utils.TestRekorURL,
-			setExperimental: true,
-			expectKeyless:   true,
+			name:          "keyless with regexp subject",
+			rekorUrl:      utils.TestRekorURL,
+			expectKeyless: true,
 			identity: cosign.Identity{
 				Issuer:        "my-issuer",
 				SubjectRegExp: "my-subject-regexp",
 			},
 		},
 		{
-			name:            "keyless with regexp issuer and subject",
-			rekorUrl:        utils.TestRekorURL,
-			setExperimental: true,
-			expectKeyless:   true,
+			name:          "keyless with regexp issuer and subject",
+			rekorUrl:      utils.TestRekorURL,
+			expectKeyless: true,
 			identity: cosign.Identity{
 				IssuerRegExp:  "my-issuer-regexp",
 				SubjectRegExp: "my-subject-regexp",
 			},
 		},
 		{
-			name:            "prioritize public key worklow",
-			rekorUrl:        utils.TestRekorURL,
-			publicKey:       utils.TestPublicKey,
-			setExperimental: true,
-			expectKeyless:   false,
+			name:          "prioritize public key worklow",
+			rekorUrl:      utils.TestRekorURL,
+			publicKey:     utils.TestPublicKey,
+			expectKeyless: false,
 			identity: cosign.Identity{
 				Issuer:  "my-issuer",
 				Subject: "my-subject",
 			},
 		},
 		{
-			name:            "keyless missing issuer",
-			setExperimental: true,
-			err:             "certificate OIDC issuer must be provided for keyless workflow",
+			name: "keyless missing issuer",
+			err:  "certificate OIDC issuer must be provided for keyless workflow",
 			identity: cosign.Identity{
 				Subject: "my-subject",
 			},
 		},
 		{
-			name:            "keyless missing subject",
-			setExperimental: true,
-			err:             "certificate identity must be provided for keyless workflow",
+			name: "keyless missing subject",
+			err:  "certificate identity must be provided for keyless workflow",
 			identity: cosign.Identity{
 				Issuer: "my-issuer",
 			},
 		},
 		{
-			name:            "keyless missing issuer in ECP",
-			setExperimental: true,
-			err:             "certificate OIDC issuer must be provided for keyless workflow",
-			policyRef:       `{"identity": {"subject": "my-subject"}}`,
+			name:      "keyless missing issuer in ECP",
+			err:       "certificate OIDC issuer must be provided for keyless workflow",
+			policyRef: `{"identity": {"subject": "my-subject"}}`,
 		},
 		{
-			name:            "keyless missing subject in ECP",
-			setExperimental: true,
-			err:             "certificate identity must be provided for keyless workflow",
-			policyRef:       `{"identity": {"issuer": "my-issuer"}}`,
+			name:      "keyless missing subject in ECP",
+			err:       "certificate identity must be provided for keyless workflow",
+			policyRef: `{"identity": {"issuer": "my-issuer"}}`,
 		},
 	}
 
@@ -361,10 +342,6 @@ func TestCheckOpts(t *testing.T) {
 			utils.SetTestFulcioRoots(t)
 			utils.SetTestCTLogPublicKey(t)
 
-			if c.setExperimental {
-				t.Setenv("EC_EXPERIMENTAL", "1")
-			}
-
 			p, err := NewPolicy(ctx, Options{
 				PolicyRef:     c.policyRef,
 				RekorURL:      c.rekorUrl,
@@ -421,7 +398,6 @@ func TestPublicKeyPEM(t *testing.T) {
 	cases := []struct {
 		name              string
 		remotePublicKey   string
-		setExperimental   bool
 		newPolicy         func(context.Context) (Policy, error)
 		expectedPublicKey string
 		err               string
@@ -436,13 +412,12 @@ func TestPublicKeyPEM(t *testing.T) {
 		{
 			name: "checkOpts is nil",
 			newPolicy: func(ctx context.Context) (Policy, error) {
-				return NewInertPolicy(ctx, "")
+				return NewInertPolicy(ctx, fmt.Sprintf(`{"publicKey": "%s"}`, utils.TestPublicKey))
 			},
 			err: "no check options or sig verifier configured",
 		},
 		{
-			name:            "keyless",
-			setExperimental: true,
+			name: "keyless",
 			newPolicy: func(ctx context.Context) (Policy, error) {
 				return NewPolicy(ctx, Options{
 					EffectiveTime: Now,
@@ -460,16 +435,11 @@ func TestPublicKeyPEM(t *testing.T) {
 			utils.SetTestFulcioRoots(t)
 			utils.SetTestCTLogPublicKey(t)
 
-			if c.setExperimental {
-				t.Setenv("EC_EXPERIMENTAL", "1")
-			}
-
 			p, err := c.newPolicy(ctx)
 			assert.NoError(t, err)
 
 			publicKeyPEM, err := p.PublicKeyPEM()
 			if c.err != "" {
-				assert.Empty(t, p)
 				assert.ErrorContains(t, err, c.err)
 				return
 			}
@@ -542,8 +512,6 @@ func TestIdentity(t *testing.T) {
 			utils.SetTestFulcioRoots(t)
 			utils.SetTestCTLogPublicKey(t)
 
-			t.Setenv("EC_EXPERIMENTAL", "1")
-
 			p, err := c.newPolicy(ctx)
 			assert.NoError(t, err)