Skip to content

removing the deprecated certification-project-id flag #1318

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 31, 2025
+86 −156
Merged

removing the deprecated certification-project-id flag #1318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 21 additions & 49 deletions cmd/preflight/cmd/check_container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ import (
"github.com/spf13/pflag"
)

var (
submit bool
componentID string
)
var submit bool

// 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 All @@ -50,7 +47,7 @@ func checkContainerCmd(runpreflight runPreflight) *cobra.Command {
Args: checkContainerPositionalArgs,
// this fmt.Sprintf is in place to keep spacing consistent with cobras two spaces that's used in: Usage, Flags, etc
Example: fmt.Sprintf(" %s", "preflight check container quay.io/repo-name/container-name:version"),
PreRunE: validateCertificationProjectID,
PreRunE: validateCertificationComponentID,
RunE: func(cmd *cobra.Command, args []string) error {
return checkContainerRunE(cmd, args, runpreflight)
},
Expand Down Expand Up @@ -86,21 +83,9 @@ 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"+
"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"+
flags.String("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")
_ = viper.BindPFlag("certification_component_id", flags.Lookup("certification-component-id"))

flags.String("platform", rt.GOARCH, "Architecture of image to pull. Defaults to runtime platform.")
_ = viper.BindPFlag("platform", flags.Lookup("platform"))
Expand Down Expand Up @@ -173,8 +158,8 @@ func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPrefl
opts...,
)

pc := lib.NewPyxisClient(ctx, cfg.CertificationProjectID, cfg.PyxisAPIToken, cfg.PyxisHost)
resultSubmitter := lib.ResolveSubmitter(pc, cfg.CertificationProjectID, cfg.DockerConfig, cfg.LogFile)
pc := lib.NewPyxisClient(ctx, cfg.CertificationComponentID, cfg.PyxisAPIToken, cfg.PyxisHost)
resultSubmitter := lib.ResolveSubmitter(pc, cfg.CertificationComponentID, cfg.DockerConfig, cfg.LogFile)

// use a noop submitter, since the konflux system has no need to submit results to pyxis.
if cfg.Konflux {
Expand Down Expand Up @@ -264,62 +249,49 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error {

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") {
if !cmd.Flag("certification-component-id").Changed && !viper.IsSet("certification_component_id") {
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") == "" {
if cmd.Flag("certification-component-id").Changed && viper.GetString("certification_component_id") == "" {
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"), "--") {
// Finally, if either certification component id or pyxis api token start with '--', it's an error
if strings.HasPrefix(viper.GetString("pyxis_api_token"), "--") || strings.HasPrefix(viper.GetString("certification_component_id"), "--") {
return fmt.Errorf("pyxis API token and certification component ID are required when --submit is present")
}
}

return nil
}

// validateCertificationProjectID validates that the certification project id is in the proper format
// validateCertificationComponentID validates that the certification component id is in the proper format
// 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 {
func validateCertificationComponentID(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, "-")
certificationComponentID := viper.GetString("certification_component_id")
// splitting the certification component id into parts. if there are more than 2 elements in the array,
// we know they inputted a legacy component id, which can not be used to query pyxis
parts := strings.Split(certificationComponentID, "-")

if len(parts) > 2 {
return fmt.Errorf("certification component 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", certificationComponentID)
}

if parts[0] == "ospid" {
viper.Set("certification_project_id", parts[1])
viper.Set("certification_component_id", parts[1])
}

return nil
Expand All @@ -328,7 +300,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.WithCertificationComponent(cfg.CertificationProjectID, cfg.PyxisAPIToken),
container.WithCertificationComponent(cfg.CertificationComponentID, 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 @@ -337,8 +309,8 @@ 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.WithCertificationComponent(cfg.CertificationProjectID, cfg.PyxisAPIToken))
if cfg.PyxisAPIToken != "" && cfg.CertificationComponentID != "" {
o = append(o, container.WithCertificationComponent(cfg.CertificationComponentID, cfg.PyxisAPIToken))
}

if cfg.Insecure {
Expand Down
76 changes: 18 additions & 58 deletions cmd/preflight/cmd/check_container_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,8 @@ var _ = Describe("Check Container Command", func() {
Expect(err).To(HaveOccurred())
Expect(out).To(ContainSubstring(errString))
},
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 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 and pyxis-api-token are not supplied", "certification component ID must be specified when --submit is present", []string{"--submit", "foo"}),
Entry("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-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"}),
Expand All @@ -200,44 +194,10 @@ 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("certcompenvid"))
})
})
When("old environment variable is used for certification ID", func() {
BeforeEach(func() {
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() {
submit = true

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("certprojenvid"))
Expect(viper.Instance().GetString("certification_component_id")).To(Equal("certcompenvid"))
})
})
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()
Expand All @@ -251,7 +211,7 @@ 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("mytoken"))
Expect(viper.Instance().GetString("certification_project_id")).To(Equal("mycomponentcertid"))
Expect(viper.Instance().GetString("certification_component_id")).To(Equal("mycomponentcertid"))
})
})
})
Expand All @@ -261,43 +221,43 @@ var _ = Describe("Check Container Command", 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")
DeferCleanup(viper.Instance().Set, "certification_project_id", "")
viper.Instance().Set("certification_component_id", "123456789")
DeferCleanup(viper.Instance().Set, "certification_component_id", "")
})
It("should not change the flag value", func() {
err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
err := validateCertificationComponentID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
Expect(err).ToNot(HaveOccurred())
Expect(viper.Instance().GetString("certification_project_id")).To(Equal("123456789"))
Expect(viper.Instance().GetString("certification_component_id")).To(Equal("123456789"))
})
})
Context("and a valid ospid format is provided", func() {
BeforeEach(func() {
viper.Instance().Set("certification_project_id", "ospid-123456789")
DeferCleanup(viper.Instance().Set, "certification_project_id", "")
viper.Instance().Set("certification_component_id", "ospid-123456789")
DeferCleanup(viper.Instance().Set, "certification_component_id", "")
})
It("should strip ospid- from the flag value", func() {
err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
err := validateCertificationComponentID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
Expect(err).ToNot(HaveOccurred())
Expect(viper.Instance().GetString("certification_project_id")).To(Equal("123456789"))
Expect(viper.Instance().GetString("certification_component_id")).To(Equal("123456789"))
})
})
Context("and a legacy format with ospid is provided", func() {
BeforeEach(func() {
viper.Instance().Set("certification_project_id", "ospid-62423-f26c346-6cc1dc7fae92")
DeferCleanup(viper.Instance().Set, "certification_project_id", "")
viper.Instance().Set("certification_component_id", "ospid-62423-f26c346-6cc1dc7fae92")
DeferCleanup(viper.Instance().Set, "certification_component_id", "")
})
It("should throw an error", func() {
err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
err := validateCertificationComponentID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
Expect(err).To(HaveOccurred())
})
})
Context("and a legacy format without ospid is provided", func() {
BeforeEach(func() {
viper.Instance().Set("certification_project_id", "62423-f26c346-6cc1dc7fae92")
DeferCleanup(viper.Instance().Set, "certification_project_id", "")
viper.Instance().Set("certification_component_id", "62423-f26c346-6cc1dc7fae92")
DeferCleanup(viper.Instance().Set, "certification_component_id", "")
})
It("should throw an error", func() {
err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
err := validateCertificationComponentID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"})
Expect(err).To(HaveOccurred())
})
})
Expand Down
2 changes: 0 additions & 2 deletions cmd/preflight/cmd/testdata/project/config.yaml
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions docs/CONFIG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ For information on how to build an index image, see [BUILDING_AN_INDEX.md](BUILD
These configurables are specific to cases where `preflight check container ...`
is called.

|Variable|Kind|Doc|Required or Optional|Default|
|--|--|--|--|--|
|`PFLT_PYXIS_HOST`|env|The Pyxis host to connect to. Must contain any additional path information leading up to the API version|optional|catalog.redhat.com/api/containers|
|`PFLT_PYXIS_API_TOKEN`|env|The API Token to be used when connecting to Pyxis. Used for authenticated calls only.|optional?|-|
|`PFLT_CERTIFICATION_PROJECT_ID`|env|Certification Project ID from connect.redhat.com. Should be supplied without the ospid- prefix.|optional?|-|
|`PFLT_DOCKERCONFIG`|env|The full path to a dockerconfigjson file, that has access to the container under test.|required|-|
| Variable |Kind| Doc |Required or Optional|Default|
|--------------------------------|--|----------------------------------------------------------------------------------------------------------|--|--|
| `PFLT_PYXIS_HOST` |env| The Pyxis host to connect to. Must contain any additional path information leading up to the API version |optional|catalog.redhat.com/api/containers|
| `PFLT_PYXIS_API_TOKEN` |env| The API Token to be used when connecting to Pyxis. Used for authenticated calls only. |optional?|-|
| `PFLT_CERTIFICATION_COMPONENT_ID` |env| Certification Component ID from connect.redhat.com. Should be supplied without the ospid- prefix. |optional?|-|
| `PFLT_DOCKERCONFIG` |env| The full path to a dockerconfigjson file, that has access to the container under test. |required|-|
2 changes: 1 addition & 1 deletion internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type commonConfig interface {
// container certification.
type containerConfig interface {
IsScratch() bool
CertificationProjectID() string
CertificationComponentID() string
PyxisHost() string
PyxisAPIToken() string
Submit() bool
Expand Down
Loading