Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add certification-component-id flag #1222

Merged
merged 1 commit into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 39 additions & 9 deletions cmd/preflight/cmd/check_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/component/view/{certification-component-id}/images\n"+
"URL paramater. This value may differ from the component PID 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")
acornett21 marked this conversation as resolved.
Show resolved Hide resolved

flags.String("platform", rt.GOARCH, "Architecture of image to pull. Defaults to runtime platform.")
_ = viper.BindPFlag("platform", flags.Lookup("platform"))
Expand Down Expand Up @@ -210,28 +225,41 @@ 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)
}
acornett21 marked this conversation as resolved.
Show resolved Hide resolved

// 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"))
}
Comment on lines +237 to +240
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this only necessary because certification_component_id is bound explicitly to env, separate from the global variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.


// --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")
}

// 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")
}

// 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")
}
}

Expand All @@ -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" {
Expand All @@ -261,7 +291,7 @@ func validateCertificationProjectID(cmd *cobra.Command, args []string) error {
// generateContainerCheckOptions returns appropriate container.Options based on cfg.
func generateContainerCheckOptions(cfg *runtime.Config) []container.Option {
o := []container.Option{
container.WithCertificationProject(cfg.CertificationProjectID, cfg.PyxisAPIToken),
container.WithCertificationComponent(cfg.CertificationProjectID, cfg.PyxisAPIToken),
container.WithDockerConfigJSONFromFile(cfg.DockerConfig),
// Always add PyxisHost, since the value is always set in viper config parsing.
container.WithPyxisHost(cfg.PyxisHost),
Expand All @@ -271,7 +301,7 @@ func generateContainerCheckOptions(cfg *runtime.Config) []container.Option {

// set auth information if both are present in config.
if cfg.PyxisAPIToken != "" && cfg.CertificationProjectID != "" {
o = append(o, container.WithCertificationProject(cfg.CertificationProjectID, cfg.PyxisAPIToken))
o = append(o, container.WithCertificationComponent(cfg.CertificationProjectID, cfg.PyxisAPIToken))
}

if cfg.Insecure {
Expand Down
88 changes: 64 additions & 24 deletions cmd/preflight/cmd/check_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions cmd/preflight/cmd/testdata/component/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pyxis_api_token: mytoken
certification_component_id: mycomponentcertid
2 changes: 2 additions & 0 deletions cmd/preflight/cmd/testdata/project/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pyxis_api_token: mytoken
certification_project_id: myprojectid
14 changes: 14 additions & 0 deletions container/check_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,21 @@ func WithDockerConfigJSONFromFile(s string) Option {
// 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.
//
// Deprecated: Use WithCertificationComponent instead
func WithCertificationProject(id, token string) Option {
return withCertificationComponent(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 withCertificationComponent(id, token)
}

func withCertificationComponent(id, token string) Option {
return func(cc *containerCheck) {
cc.pyxisToken = token
cc.certificationProjectID = id
Expand Down
Loading