From e6664bb575485d618806a2646f671df3787b4976 Mon Sep 17 00:00:00 2001 From: "Brad P. Crochet" Date: Mon, 9 Dec 2024 11:52:43 -0500 Subject: [PATCH] Add certification-component-id flag This flag deprecates the certification-project-id flag. The field has been changed on the server side, and in certification documentation. Fixes #1185 Signed-off-by: Brad P. Crochet --- cmd/preflight/cmd/check_container.go | 44 ++++++++++-- cmd/preflight/cmd/check_container_test.go | 88 ++++++++++++++++------- container/check_container.go | 13 ++++ docs/RECIPES.md | 14 ++-- internal/viper/viper.go | 8 +++ 5 files changed, 129 insertions(+), 38 deletions(-) diff --git a/cmd/preflight/cmd/check_container.go b/cmd/preflight/cmd/check_container.go index e30c0e4c..453f528a 100644 --- a/cmd/preflight/cmd/check_container.go +++ b/cmd/preflight/cmd/check_container.go @@ -33,7 +33,10 @@ import ( "github.com/spf13/pflag" ) -var submit bool +var ( + submit bool + componentID string +) // runPreflight is introduced to make testing of this command possible, it has the same method signature as cli.RunPreflight. type runPreflight func(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error @@ -82,9 +85,21 @@ func checkContainerCmd(runpreflight runPreflight) *cobra.Command { flags.String("pyxis-env", check.DefaultPyxisEnv, "Env to use for Pyxis submissions.") _ = viper.BindPFlag("pyxis_env", flags.Lookup("pyxis-env")) - flags.String("certification-project-id", "", fmt.Sprintf("Certification Project ID from connect.redhat.com/projects/{certification-project-id}/overview\n"+ + flags.String("certification-project-id", "", fmt.Sprintf("Certification project ID from connect.redhat.com/projects/{certification-project-id}/overview\n"+ "URL paramater. This value may differ from the PID on the overview page. (env: PFLT_CERTIFICATION_PROJECT_ID)")) _ = viper.BindPFlag("certification_project_id", flags.Lookup("certification-project-id")) + _ = flags.MarkDeprecated("certification-project-id", "please use --certification-component-id instead") + + // Use a bound package-level var here. We are going to leave the rest of the code + // using the Viper id of 'certification_project_id' in order to minimize changes + // in the overall code base. + // When --certification-project-id is fully removed, this should become bound in Viper + flags.StringVar(&componentID, "certification-component-id", "", fmt.Sprintf("Certification component ID from connect.redhat.com/projects/{certification-component-id}/overview\n"+ + "URL paramater. This value may differ from the CID on the overview page. (env: PFLT_CERTIFICATION_COMPONENT_ID)")) + // Here, we are forcing an env binding, so we can check that later. This should also + // be moved to "automatic" once the old project id is removed + _ = viper.BindEnv("certification_component_id") + checkContainerCmd.MarkFlagsMutuallyExclusive("certification-project-id", "certification-component-id") flags.String("platform", rt.GOARCH, "Architecture of image to pull. Defaults to runtime platform.") _ = viper.BindPFlag("platform", flags.Lookup("platform")) @@ -210,12 +225,25 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { } }) - // --submit was specified viper := viper.Instance() + + // If the new flag is set, use that + if cmd.Flag("certification-component-id").Changed { + cmd.Flag("certification-project-id").Changed = true + cmd.Flag("certification-component-id").Changed = false + viper.Set("certification_project_id", componentID) + } + + // However, if the new env var is set, that's the priority + if viper.IsSet("certification_component_id") { + viper.Set("certification_project_id", viper.GetString("certification_component_id")) + } + + // --submit was specified if submit { // If the flag is not marked as changed AND viper hasn't gotten it from environment, it's an error if !cmd.Flag("certification-project-id").Changed && !viper.IsSet("certification_project_id") { - return fmt.Errorf("certification Project ID must be specified when --submit is present") + return fmt.Errorf("certification component ID must be specified when --submit is present") } if !cmd.Flag("pyxis-api-token").Changed && !viper.IsSet("pyxis_api_token") { return fmt.Errorf("pyxis API Token must be specified when --submit is present") @@ -223,7 +251,7 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { // If the flag is marked as changed AND it's still empty, it's an error if cmd.Flag("certification-project-id").Changed && viper.GetString("certification_project_id") == "" { - return fmt.Errorf("certification Project ID cannot be empty when --submit is present") + return fmt.Errorf("certification component ID cannot be empty when --submit is present") } if cmd.Flag("pyxis-api-token").Changed && viper.GetString("pyxis_api_token") == "" { return fmt.Errorf("pyxis API Token cannot be empty when --submit is present") @@ -231,7 +259,7 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { // Finally, if either certification project id or pyxis api token start with '--', it's an error if strings.HasPrefix(viper.GetString("pyxis_api_token"), "--") || strings.HasPrefix(viper.GetString("certification_project_id"), "--") { - return fmt.Errorf("pyxis API token and certification ID are required when --submit is present") + return fmt.Errorf("pyxis API token and certification component ID are required when --submit is present") } } @@ -242,13 +270,15 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { // and throws an error if the value provided is in a legacy format that is not usable to query pyxis func validateCertificationProjectID(cmd *cobra.Command, args []string) error { viper := viper.Instance() + + // From here on out, we just treat project ID like we did before. certificationProjectID := viper.GetString("certification_project_id") // splitting the certification project id into parts. if there are more than 2 elements in the array, // we know they inputted a legacy project id, which can not be used to query pyxis parts := strings.Split(certificationProjectID, "-") if len(parts) > 2 { - return fmt.Errorf("certification project id: %s is improperly formatted see help command for instructions on obtaining proper value", certificationProjectID) + return fmt.Errorf("certification component id: %s is improperly formatted see help command for instructions on obtaining proper value", certificationProjectID) } if parts[0] == "ospid" { diff --git a/cmd/preflight/cmd/check_container_test.go b/cmd/preflight/cmd/check_container_test.go index b4454743..7cdf538a 100644 --- a/cmd/preflight/cmd/check_container_test.go +++ b/cmd/preflight/cmd/check_container_test.go @@ -72,6 +72,7 @@ var _ = Describe("Check Container Command", func() { var s *httptest.Server var u *url.URL BeforeEach(func() { + viper.Reset() manifests = make(map[string]string, 2) // Set up a fake registry. registryLogger := log.New(io.Discard, "", log.Ldate) @@ -162,26 +163,35 @@ var _ = Describe("Check Container Command", func() { DescribeTable("and the user has enabled the submit flag", func(errString string, args []string) { + viper.Reset() out, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil), args...) Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring(errString)) }, - Entry("certification-project-id and pyxis-api-token are not supplied", "certification Project ID must be specified when --submit is present", []string{"--submit", "foo"}), - Entry("pyxis-api-token is not supplied", "pyxis API Token must be specified when --submit is present", []string{"foo", "--submit", "--certification-project-id=fooid"}), - Entry("certification-project-id is not supplied", "certification Project ID must be specified when --submit is present", []string{"--submit", "foo", "--pyxis-api-token=footoken"}), - Entry("pyxis-api-token flag is present but empty because of '='", "cannot be empty when --submit is present", []string{"foo", "--submit", "--certification-project-id=fooid", "--pyxis-api-token="}), + Entry("certification-project-id or certification-component-id and pyxis-api-token are not supplied", "certification component ID must be specified when --submit is present", []string{"--submit", "foo"}), + Entry("certification-project-id is supplied and pyxis-api-token is not supplied", "pyxis API Token must be specified when --submit is present", []string{"foo", "--submit", "--certification-project-id=fooid"}), + Entry("certification-project-id or certification-component-id is not supplied", "certification component ID must be specified when --submit is present", []string{"--submit", "foo", "--pyxis-api-token=footoken"}), + Entry("certification-project-id and pyxis-api-token flag is present but empty because of '='", "cannot be empty when --submit is present", []string{"foo", "--submit", "--certification-project-id=fooid", "--pyxis-api-token="}), Entry("certification-project-id flag is present but empty because of '='", "cannot be empty when --submit is present", []string{"foo", "--submit", "--certification-project-id=", "--pyxis-api-token=footoken"}), - Entry("submit is passed after empty api token", "pyxis API token and certification ID are required when --submit is present", []string{"foo", "--certification-project-id=fooid", "--pyxis-api-token", "--submit"}), - Entry("submit is passed with explicit value after empty api token", "pyxis API token and certification ID are required when --submit is present", []string{"foo", "--certification-project-id=fooid", "--pyxis-api-token", "--submit=true"}), - Entry("submit is passed and insecure is specified", "if any flags in the group [submit insecure] are set", []string{"foo", "--submit", "--insecure", "--certification-project-id=fooid", "--pyxis-api-token=footoken"}), + Entry("submit is passed after empty api token with certification-project-id", "pyxis API token and certification component ID are required when --submit is present", []string{"foo", "--certification-project-id=fooid", "--pyxis-api-token", "--submit"}), + Entry("certification-project-id and submit is passed with explicit value after empty api token", "pyxis API token and certification component ID are required when --submit is present", []string{"foo", "--certification-project-id=fooid", "--pyxis-api-token", "--submit=true"}), + Entry("certification-project-id and submit is passed and insecure is specified", "if any flags in the group [submit insecure] are set", []string{"foo", "--submit", "--insecure", "--certification-project-id=fooid", "--pyxis-api-token=footoken"}), + Entry("certification-component-id is supplied and pyxis-api-token is not supplied", "pyxis API Token must be specified when --submit is present", []string{"foo", "--submit", "--certification-component-id=fooid"}), + Entry("certification-component-id and pyxis-api-token flag is present but empty because of '='", "cannot be empty when --submit is present", []string{"foo", "--submit", "--certification-component-id=fooid", "--pyxis-api-token="}), + Entry("certification-component-id flag is present but empty because of '='", "cannot be empty when --submit is present", []string{"foo", "--submit", "--certification-component-id=", "--pyxis-api-token=footoken"}), + Entry("submit is passed after empty api token with certification-component-id", "pyxis API token and certification component ID are required when --submit is present", []string{"foo", "--certification-component-id=fooid", "--pyxis-api-token", "--submit"}), + Entry("certification-component-id and submit is passed with explicit value after empty api token", "pyxis API token and certification component ID are required when --submit is present", []string{"foo", "--certification-component-id=fooid", "--pyxis-api-token", "--submit=true"}), + Entry("certification-component-id and submit is passed and insecure is specified", "if any flags in the group [submit insecure] are set", []string{"foo", "--submit", "--insecure", "--certification-component-id=fooid", "--pyxis-api-token=footoken"}), ) When("the user enables the submit flag", func() { When("environment variables are used for certification ID and api token", func() { BeforeEach(func() { - os.Setenv("PFLT_CERTIFICATION_PROJECT_ID", "certid") + viper.Reset() + initConfig(viper.Instance()) + os.Setenv("PFLT_CERTIFICATION_COMPONENT_ID", "certcompenvid") os.Setenv("PFLT_PYXIS_API_TOKEN", "tokenid") - DeferCleanup(os.Unsetenv, "PFLT_CERTIFICATION_PROJECT_ID") + DeferCleanup(os.Unsetenv, "PFLT_CERTIFICATION_COMPONENT_ID") DeferCleanup(os.Unsetenv, "PFLT_PYXIS_API_TOKEN") }) It("should still execute with no error", func() { @@ -190,35 +200,65 @@ var _ = Describe("Check Container Command", func() { err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) Expect(err).ToNot(HaveOccurred()) Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("tokenid")) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("certid")) + Expect(viper.Instance().GetString("certification_project_id")).To(Equal("certcompenvid")) }) }) - When("a config file is used", func() { + When("old environment variable is used for certification ID", func() { BeforeEach(func() { - config := `pyxis_api_token: mytoken -certification_project_id: mycertid` - tempDir, err := os.MkdirTemp("", "check-container-submit-*") - Expect(err).ToNot(HaveOccurred()) - err = os.WriteFile(filepath.Join(tempDir, "config.yaml"), bytes.NewBufferString(config).Bytes(), 0o644) - Expect(err).ToNot(HaveOccurred()) - viper.Instance().AddConfigPath(tempDir) - DeferCleanup(os.RemoveAll, tempDir) + viper.Reset() + initConfig(viper.Instance()) + os.Setenv("PFLT_CERTIFICATION_PROJECT_ID", "certprojenvid") + os.Setenv("PFLT_PYXIS_API_TOKEN", "tokenid") + DeferCleanup(os.Unsetenv, "PFLT_CERTIFICATION_PROJECT_ID") + DeferCleanup(os.Unsetenv, "PFLT_PYXIS_API_TOKEN") }) It("should still execute with no error", func() { - // Make sure that we've read the config file - initConfig(viper.Instance()) submit = true err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) Expect(err).ToNot(HaveOccurred()) - Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("mytoken")) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("mycertid")) + Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("tokenid")) + Expect(viper.Instance().GetString("certification_project_id")).To(Equal("certprojenvid")) + }) + }) + When("a config file is used", func() { + When("the deprecated certification_project_id config file is used", func() { + JustBeforeEach(func() { + viper.Reset() + viper.Instance().AddConfigPath("testdata/project/") + }) + It("should still execute with no error", func() { + // Make sure that we've read the config file + initConfig(viper.Instance()) + submit = true + + err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) + Expect(err).ToNot(HaveOccurred()) + Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("mytoken")) + Expect(viper.Instance().GetString("certification_project_id")).To(Equal("myprojectid")) + }) + }) + When("certification_component_id config file is used", func() { + JustBeforeEach(func() { + viper.Reset() + viper.Instance().AddConfigPath("testdata/component/") + }) + It("should still execute with no error", func() { + // Make sure that we've read the config file + initConfig(viper.Instance()) + submit = true + + err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) + Expect(err).ToNot(HaveOccurred()) + Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("mytoken")) + Expect(viper.Instance().GetString("certification_project_id")).To(Equal("mycomponentcertid")) + }) }) }) }) }) - Context("When validating the certification-project-id flag", func() { + Context("When validating the certification-component-id flag", func() { Context("and the flag is set properly", func() { BeforeEach(func() { viper.Instance().Set("certification_project_id", "123456789") diff --git a/container/check_container.go b/container/check_container.go index 77bde6c0..28014897 100644 --- a/container/check_container.go +++ b/container/check_container.go @@ -125,11 +125,24 @@ func WithDockerConfigJSONFromFile(s string) Option { } } +// DEPRECATED: Use WithCertificationProject instead // WithCertificationProject adds the project's id and pyxis token to the check // allowing for the project's metadata to change the certification (if necessary). // An example might be the Scratch or Privileged flags on a project allowing for // the corresponding policy to be executed. func WithCertificationProject(id, token string) Option { + return withCertificationProject(id, token) +} + +// WithCertificationComponent adds the component's id and pyxis token to the check +// allowing for the copmonent's metadata to change the certification (if necessary). +// An example might be the Scratch or Privileged flags on a project allowing for +// the corresponding policy to be executed. +func WithCertificationComponent(id, token string) Option { + return withCertificationProject(id, token) +} + +func withCertificationProject(id, token string) Option { return func(cc *containerCheck) { cc.pyxisToken = token cc.certificationProjectID = id diff --git a/docs/RECIPES.md b/docs/RECIPES.md index c87e4c4c..ae5a4455 100644 --- a/docs/RECIPES.md +++ b/docs/RECIPES.md @@ -147,10 +147,10 @@ These examples are shown using the Container policy against a container image You will also need: - Your container image published to a container registry - An example would be `quay.io/repo-name/container-name:version` -- A Certification Project ID of the project that was set up in Red Hat Partner Connect +- A Certification Component ID of the component that was set up in Red Hat Partner Connect - This value can be obtained from the Overview page's URL - - For the following example Overview URL of `https://connect.redhat.com/projects/1234567890aabbccddeeffgg/overview` - - The Certification Project ID would be: `1234567890aabbccddeeffgg` + - For the following example Overview URL of `https://connect.redhat.com/components/1234567890aabbccddeeffgg/overview` + - The Certification Component ID would be: `1234567890aabbccddeeffgg` - Required for submit - A Partner Connect API Key - An API Key can be created in Red Hat Partner Connect at the following [URL](https://connect.redhat.com/account/api-keys) @@ -174,7 +174,7 @@ Running container policy checks against a container that has passed all tests an preflight check container registry.example.org/your-namespace/your-image:sometag \ --submit \ --pyxis-api-token=abcdefghijklmnopqrstuvwxyz123456 \ ---certification-project-id=1234567890a987654321bcde \ +--certification-component-id=1234567890a987654321bcde \ --docker-config=/path/to/your/dockerconfig ``` @@ -188,7 +188,7 @@ loglevel: trace logfile: artifacts/preflight.log artifacts: artifacts junit: true -certification_project_id: my_nice_project_id +certification_component_id: my_nice_component_id pyxis_api_token: my_nice_token ``` @@ -208,7 +208,7 @@ artifacts and logfiles to be written by using the `PFLT_ARTIFACTS` and `PFLT_LOGFILE` environment variables. Then we bind host volumes to these locations so that the data will be preserved when the container completes (the container will be deleted after completion due to the `--rm` flag). We also make -the assumption that the user would like to submit results to Red Hat; meaning `PFLT_CERTIFICATION_PROJECT_ID`, +the assumption that the user would like to submit results to Red Hat; meaning `PFLT_CERTIFICATION_COMPONENT_ID`, `PFLT_PYXIS_API_TOKEN` and `PFLT_DOCKERCONFIG` need to be provided. **Note:** The docker config provided to the `PFLT_DOCKERCONFIG` environment should be from the following command: `podman login --username [USERNAME] --password [PASSWORD] --authfile ./temp-authfile.json [REGISTRY]` @@ -222,7 +222,7 @@ $CONTAINER_TOOL run \ --env PFLT_LOGLEVEL=trace \ --env PFLT_ARTIFACTS=/artifacts \ --env PFLT_LOGFILE=/artifacts/preflight.log \ - --env PFLT_CERTIFICATION_PROJECT_ID=1234567890a987654321bcde \ + --env PFLT_CERTIFICATION_COMPONENT_ID=1234567890a987654321bcde \ --env PFLT_PYXIS_API_TOKEN=abcdefghijklmnopqrstuvwxyz123456 \ --env PFLT_DOCKERCONFIG=/temp-authfile.json \ -v /some/path/on/your/host/artifacts:/artifacts \ diff --git a/internal/viper/viper.go b/internal/viper/viper.go index 9815ab88..1d956cea 100644 --- a/internal/viper/viper.go +++ b/internal/viper/viper.go @@ -27,3 +27,11 @@ func Instance() *spfviper.Viper { } return instance } + +// Reset creates a new Viper instance. This should really only be used +// for testing purposes. +func Reset() { + mu.Lock() + defer mu.Unlock() + instance = spfviper.New() +}