Skip to content

Commit

Permalink
adding logic to exit early and display the help if the user gives a l…
Browse files Browse the repository at this point in the history
…egacy project id

Signed-off-by: Adam D. Cornett <adc@redhat.com>
  • Loading branch information
acornett21 authored and jomkz committed Aug 30, 2022
1 parent ba153f4 commit 7555dc9
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 18 deletions.
9 changes: 1 addition & 8 deletions certification/runtime/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package runtime

import (
"os"
"strings"

"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/policy"

Expand Down Expand Up @@ -63,13 +62,7 @@ func (c *Config) storeContainerPolicyConfiguration(vcfg viper.Viper) {
c.PyxisAPIToken = vcfg.GetString("pyxis_api_token")
c.Submit = vcfg.GetBool("submit")
c.PyxisHost = pyxisHostLookup(vcfg.GetString("pyxis_env"), vcfg.GetString("pyxis_host"))

// Strip the ospid- prefix from the project ID if provided.
certificationProjectID := vcfg.GetString("certification_project_id")
if strings.HasPrefix(certificationProjectID, "ospid-") {
certificationProjectID = strings.Split(certificationProjectID, "-")[1]
}
c.CertificationProjectID = certificationProjectID
c.CertificationProjectID = vcfg.GetString("certification_project_id")
}

// storeOperatorPolicyConfiguration reads operator-policy-specific config
Expand Down
10 changes: 0 additions & 10 deletions certification/runtime/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,6 @@ var _ = Describe("Viper to Runtime Config", func() {
})
})

Context("With proper values, but with an opsid- prefix on the certification project ID", func() {
It("should properly trim the ospid- prefix and succeed", func() {
baseViperCfg.Set("certification_project_id", "ospid-111111111111")
expectedRuntimeCfg.CertificationProjectID = "111111111111"
cfg, err := NewConfigFrom(*baseViperCfg)
Expect(err).ToNot(HaveOccurred())
Expect(*cfg).To(BeEquivalentTo(*expectedRuntimeCfg))
})
})

It("should only have 20 struct keys for tests to be valid", func() {
// If this test fails, it means a developer has added or removed
// keys from runtime.Config, and so these tests may no longer be
Expand Down
21 changes: 21 additions & 0 deletions cmd/check_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"context"
"errors"
"fmt"
"strings"

Expand All @@ -28,6 +29,7 @@ func checkContainerCmd() *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,
RunE: checkContainerRunE,
}

Expand Down Expand Up @@ -208,3 +210,22 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error {

return nil
}

// validateCertificationProjectId validates that the certification project 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 {
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 errors.New(fmt.Sprintf("certification project id: %s is improperly formatted see help command for instructions on obtaining proper value", certificationProjectID))
}

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

return nil
}
41 changes: 41 additions & 0 deletions cmd/check_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,4 +529,45 @@ certification_project_id: mycertid`
})
})
})

Context("When validating the certification-project-id flag", func() {
Context("and the flag is set properly", func() {
BeforeEach(func() {
viper.Set("certification_project_id", "123456789")
})
It("should not change the flag value", func() {
err := validateCertificationProjectId(checkContainerCmd(), []string{"foo"})
Expect(err).ToNot(HaveOccurred())
Expect(viper.GetString("certification_project_id")).To(Equal("123456789"))
})
})
Context("and a valid ospid format is provided", func() {
BeforeEach(func() {
viper.Set("certification_project_id", "ospid-123456789")
})
It("should strip ospid- from the flag value", func() {
err := validateCertificationProjectId(checkContainerCmd(), []string{"foo"})
Expect(err).ToNot(HaveOccurred())
Expect(viper.GetString("certification_project_id")).To(Equal("123456789"))
})
})
Context("and a legacy format with ospid is provided", func() {
BeforeEach(func() {
viper.Set("certification_project_id", "ospid-62423-f26c346-6cc1dc7fae92")
})
It("should throw an error", func() {
err := validateCertificationProjectId(checkContainerCmd(), []string{"foo"})
Expect(err).To(HaveOccurred())
})
})
Context("and a legacy format without ospid is provided", func() {
BeforeEach(func() {
viper.Set("certification_project_id", "62423-f26c346-6cc1dc7fae92")
})
It("should throw an error", func() {
err := validateCertificationProjectId(checkContainerCmd(), []string{"foo"})
Expect(err).To(HaveOccurred())
})
})
})
})

0 comments on commit 7555dc9

Please sign in to comment.