diff --git a/container/check_container_test.go b/container/check_container_test.go index 5f81a6a7..be282e54 100644 --- a/container/check_container_test.go +++ b/container/check_container_test.go @@ -70,7 +70,7 @@ var _ = Describe("Container Check Execution", func() { Expect(err).ToNot(HaveOccurred()) Expect(chk.policy).To(Equal("container")) Expect(chk.resolved).To(Equal(true)) - Expect(len(chk.checks)).To(Equal(8)) + Expect(len(chk.checks)).To(Equal(9)) }) It("Should list checks without issue", func() { @@ -78,7 +78,7 @@ var _ = Describe("Container Check Execution", func() { policy, checks, err := chk.List(ctx) Expect(err).ToNot(HaveOccurred()) Expect(policy).To(Equal("container")) - Expect(len(checks)).To(Equal(8)) + Expect(len(checks)).To(Equal(9)) }) It("Should run without issue", func() { diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 76ff5652..7ef258b9 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -765,6 +765,7 @@ func InitializeContainerChecks(ctx context.Context, p policy.Policy, cfg Contain cfg.PyxisAPIToken, cfg.CertificationProjectID, &http.Client{Timeout: 60 * time.Second})), + &containerpol.HasProhibitedContainerName{}, }, nil case policy.PolicyRoot: return []check.Check{ @@ -779,6 +780,7 @@ func InitializeContainerChecks(ctx context.Context, p policy.Policy, cfg Contain cfg.PyxisAPIToken, cfg.CertificationProjectID, &http.Client{Timeout: 60 * time.Second})), + &containerpol.HasProhibitedContainerName{}, }, nil case policy.PolicyScratchNonRoot: return []check.Check{ @@ -787,6 +789,7 @@ func InitializeContainerChecks(ctx context.Context, p policy.Policy, cfg Contain &containerpol.MaxLayersCheck{}, &containerpol.HasRequiredLabelsCheck{}, &containerpol.RunAsNonRootCheck{}, + &containerpol.HasProhibitedContainerName{}, }, nil case policy.PolicyScratchRoot: return []check.Check{ @@ -794,6 +797,7 @@ func InitializeContainerChecks(ctx context.Context, p policy.Policy, cfg Contain containerpol.NewHasUniqueTagCheck(cfg.DockerConfig), &containerpol.MaxLayersCheck{}, &containerpol.HasRequiredLabelsCheck{}, + &containerpol.HasProhibitedContainerName{}, }, nil } diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index eb54129a..100c2fd7 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -345,6 +345,7 @@ var _ = Describe("Check Name Queries", func() { "RunAsNonRoot", "HasModifiedFiles", "BasedOnUbi", + "HasProhibitedContainerName", }), Entry("default operator policy", OperatorPolicy, []string{ "ScorecardBasicSpecCheck", @@ -363,12 +364,14 @@ var _ = Describe("Check Name Queries", func() { "LayerCountAcceptable", "HasRequiredLabel", "RunAsNonRoot", + "HasProhibitedContainerName", }), Entry("scratch container policy", ScratchRootContainerPolicy, []string{ "HasLicense", "HasUniqueTag", "LayerCountAcceptable", "HasRequiredLabel", + "HasProhibitedContainerName", }), Entry("root container policy", RootExceptionContainerPolicy, []string{ "HasLicense", @@ -378,6 +381,7 @@ var _ = Describe("Check Name Queries", func() { "HasRequiredLabel", "HasModifiedFiles", "BasedOnUbi", + "HasProhibitedContainerName", }), ) diff --git a/internal/policy/container/has_prohibited_container_name.go b/internal/policy/container/has_prohibited_container_name.go new file mode 100644 index 00000000..eeb872c9 --- /dev/null +++ b/internal/policy/container/has_prohibited_container_name.go @@ -0,0 +1,57 @@ +package container + +import ( + "context" + "strings" + + "github.com/go-logr/logr" + + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/check" + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/image" + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/log" +) + +var _ check.Check = &HasProhibitedContainerName{} + +type HasProhibitedContainerName struct{} + +func (p HasProhibitedContainerName) Validate(ctx context.Context, imageReference image.ImageReference) (result bool, err error) { + return p.validate(ctx, p.getDataForValidate(imageReference.ImageRepository)) +} + +func (p HasProhibitedContainerName) getDataForValidate(imageRepository string) string { + // splitting on '/' to get container name, at this point we know that + // crane's ParseReference has set ImageReference.imageRepository in a valid format + return strings.Split(imageRepository, "/")[1] +} + +func (p HasProhibitedContainerName) validate(ctx context.Context, containerName string) (bool, error) { + logger := logr.FromContextOrDiscard(ctx) + + if violatesRedHatTrademark(containerName) { + logger.V(log.DBG).Info("container name violate Red Hat trademark", "container-name", containerName) + return false, nil + } + + return true, nil +} + +func (p HasProhibitedContainerName) Name() string { + return "HasProhibitedContainerName" +} + +func (p HasProhibitedContainerName) Metadata() check.Metadata { + return check.Metadata{ + Description: "Checking if the container-name violates Red Hat trademark.", + Level: "good", + KnowledgeBaseURL: certDocumentationURL, + CheckURL: certDocumentationURL, + } +} + +func (p HasProhibitedContainerName) Help() check.HelpText { + return check.HelpText{ + Message: "Check HasProhibitedContainerName encountered an error. Please review the preflight.log file for more information.", + Suggestion: "Update container-name ie (quay.io/repo-name/container-name) to not violate Red Hat trademark.", + } +} diff --git a/internal/policy/container/has_prohibited_container_name_test.go b/internal/policy/container/has_prohibited_container_name_test.go new file mode 100644 index 00000000..ec30f749 --- /dev/null +++ b/internal/policy/container/has_prohibited_container_name_test.go @@ -0,0 +1,42 @@ +package container + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/image" +) + +var _ = Describe("HasProhibitedContainerName", func() { + var ( + hasProhibitedContainerName HasProhibitedContainerName + imageRef image.ImageReference + ) + + Describe("Checking for trademark violations", func() { + Context("When a container name does not violate trademark", func() { + BeforeEach(func() { + imageRef.ImageRepository = "opdev/simple-demo-operator" + }) + It("should pass Validate", func() { + ok, err := hasProhibitedContainerName.Validate(context.TODO(), imageRef) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + }) + }) + Context("When a container name violates trademark", func() { + BeforeEach(func() { + imageRef.ImageRepository = "opdev/red-hat-container" + }) + It("should not pass Validate", func() { + ok, err := hasProhibitedContainerName.Validate(context.TODO(), imageRef) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) + }) + + AssertMetaData(&hasProhibitedContainerName) +}) diff --git a/internal/policy/container/has_required_labels.go b/internal/policy/container/has_required_labels.go index 2c9a9ff0..5a0a4284 100644 --- a/internal/policy/container/has_required_labels.go +++ b/internal/policy/container/has_required_labels.go @@ -12,7 +12,9 @@ import ( cranev1 "github.com/google/go-containerregistry/pkg/v1" ) -var requiredLabels = []string{"name", "vendor", "version", "release", "summary", "description"} +var requiredLabels = []string{"name", "vendor", "version", "release", "summary", "description", "maintainer"} + +var trademarkLabels = []string{"name", "vendor", "maintainer"} var _ check.Check = &HasRequiredLabelsCheck{} @@ -37,6 +39,13 @@ func (p *HasRequiredLabelsCheck) getDataForValidate(image cranev1.Image) (map[st func (p *HasRequiredLabelsCheck) validate(ctx context.Context, labels map[string]string) (bool, error) { logger := logr.FromContextOrDiscard(ctx) + trademarkViolationLabels := []string{} + for _, label := range trademarkLabels { + if violatesRedHatTrademark(labels[label]) { + trademarkViolationLabels = append(trademarkViolationLabels, label) + } + } + missingLabels := []string{} for _, label := range requiredLabels { if labels[label] == "" { @@ -49,7 +58,11 @@ func (p *HasRequiredLabelsCheck) validate(ctx context.Context, labels map[string logger.V(log.DBG).Info("expected labels are missing", "missingLabels", missingLabels) } - return len(missingLabels) == 0, nil + if len(trademarkViolationLabels) > 0 { + logger.V(log.DBG).Info("labels violate Red Hat trademark", "trademarkViolationLabels", trademarkViolationLabels) + } + + return len(missingLabels) == 0 && len(trademarkViolationLabels) == 0, nil } func (p *HasRequiredLabelsCheck) Name() string { @@ -58,7 +71,8 @@ func (p *HasRequiredLabelsCheck) Name() string { func (p *HasRequiredLabelsCheck) Metadata() check.Metadata { return check.Metadata{ - Description: "Checking if the required labels (name, vendor, version, release, summary, description) are present in the container metadata.", + Description: "Checking if the required labels (name, vendor, version, release, summary, description, maintainer) are present in the container metadata." + + "and that they do not violate Red Hat trademark.", Level: "good", KnowledgeBaseURL: certDocumentationURL, CheckURL: certDocumentationURL, @@ -67,7 +81,8 @@ func (p *HasRequiredLabelsCheck) Metadata() check.Metadata { func (p *HasRequiredLabelsCheck) Help() check.HelpText { return check.HelpText{ - Message: "Check Check HasRequiredLabel encountered an error. Please review the preflight.log file for more information.", - Suggestion: "Add the following labels to your Dockerfile or Containerfile: name, vendor, version, release, summary, description", + Message: "Check HasRequiredLabel encountered an error. Please review the preflight.log file for more information.", + Suggestion: "Add the following labels to your Dockerfile or Containerfile: name, vendor, version, release, summary, description, maintainer" + + "and validate that they do not violate Red Hat trademark.", } } diff --git a/internal/policy/container/has_required_labels_test.go b/internal/policy/container/has_required_labels_test.go index 263d7379..c28fa4c8 100644 --- a/internal/policy/container/has_required_labels_test.go +++ b/internal/policy/container/has_required_labels_test.go @@ -11,9 +11,10 @@ import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/image" ) -func getLabels(bad bool) map[string]string { +func getLabels(override string) map[string]string { labels := map[string]string{ - "name": "name", + "name": "Something for Red Hat OpenShift", + "maintainer": "maintainer", "vendor": "vendor", "version": "version", "release": "release", @@ -21,8 +22,11 @@ func getLabels(bad bool) map[string]string { "description": "description", } - if bad { + switch override { + case "remove-label": delete(labels, "description") + case "violates-trademark": + labels["name"] = "Red Hat" } return labels @@ -31,15 +35,23 @@ func getLabels(bad bool) map[string]string { func getConfigFile() (*cranev1.ConfigFile, error) { return &cranev1.ConfigFile{ Config: cranev1.Config{ - Labels: getLabels(false), + Labels: getLabels(""), }, }, nil } -func getBadConfigFile() (*cranev1.ConfigFile, error) { +func getRemoveLabelConfigFile() (*cranev1.ConfigFile, error) { return &cranev1.ConfigFile{ Config: cranev1.Config{ - Labels: getLabels(true), + Labels: getLabels("remove-label"), + }, + }, nil +} + +func getViolatesTrademarkConfigFile() (*cranev1.ConfigFile, error) { + return &cranev1.ConfigFile{ + Config: cranev1.Config{ + Labels: getLabels("violates-trademark"), }, }, nil } @@ -68,7 +80,7 @@ var _ = Describe("HasRequiredLabels", func() { Context("When it does not have required labels", func() { BeforeEach(func() { fakeImage := fakecranev1.FakeImage{ - ConfigFileStub: getBadConfigFile, + ConfigFileStub: getRemoveLabelConfigFile, } imageRef.ImageInfo = &fakeImage }) @@ -78,6 +90,19 @@ var _ = Describe("HasRequiredLabels", func() { Expect(ok).To(BeFalse()) }) }) + Context("When label.name violates Red Hat Trademark", func() { + BeforeEach(func() { + fakeImage := fakecranev1.FakeImage{ + ConfigFileStub: getViolatesTrademarkConfigFile, + } + imageRef.ImageInfo = &fakeImage + }) + It("should not succeed the check and throw an error", func() { + ok, err := hasRequiredLabelsCheck.Validate(context.TODO(), imageRef) + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeFalse()) + }) + }) }) AssertMetaData(&hasRequiredLabelsCheck) diff --git a/internal/policy/container/trademark_validator.go b/internal/policy/container/trademark_validator.go new file mode 100644 index 00000000..38a9468d --- /dev/null +++ b/internal/policy/container/trademark_validator.go @@ -0,0 +1,32 @@ +package container + +import ( + "regexp" + "strings" +) + +// violatesRedHatTrademark validates if a string meets specific "Red Hat" naming criteria +func violatesRedHatTrademark(s string) bool { + // string starts with Red Hat variant + startingWithRedHat := regexp.MustCompile("^[^a-z0-9]*red[^a-z0-9]*hat").MatchString(strings.ToLower(s)) + + // string contain Red Hat variant (not starting with) + containsRedHat := len(regexp.MustCompile("red[^a-z0-9]*hat").FindAllString(strings.ToLower(s), -1)) + + // string contains "for Red Hat" variant + containsForRedHat := regexp.MustCompile("for[^a-z0-9]*red[^a-z0-9]*hat").MatchString(strings.ToLower(s)) + + // We explicitly fail for this, so we don't need to count it here. + if startingWithRedHat { + containsRedHat -= 1 + } + + // This is acceptable, so we don't count it against the string. + if containsForRedHat { + containsRedHat -= 1 + } + + containsInvalidReference := containsRedHat > 0 + + return startingWithRedHat || containsInvalidReference +} diff --git a/internal/policy/container/trademark_validator_test.go b/internal/policy/container/trademark_validator_test.go new file mode 100644 index 00000000..d79754cf --- /dev/null +++ b/internal/policy/container/trademark_validator_test.go @@ -0,0 +1,26 @@ +package container + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("TrademarkValidator", func() { + DescribeTable("Test all presentations of `Red Hat`", + func(trademarkText string, expected bool) { + result := violatesRedHatTrademark(trademarkText) + Expect(result).To(Equal(expected)) + }, + + Entry("`Red Hat` should violate trademark policy", "Red Hat", true), + Entry("`Something for Red Hat OpenShift` should not violate trademark policy", "Something for Red Hat OpenShift", false), + Entry("`Red-Hat` should violate trademark policy", "Red-Hat", true), + Entry("`Red_Hat` should violate trademark policy", "Red_Hat", true), + Entry("`For-Red-Hat` should not violate trademark policy", "For-Red-Hat", false), + Entry("`For_Red_Hat` should not violate trademark policy", "For_Red_Hat", false), + Entry("`RED HAT ` should violate trademark policy", "RED HAT ", true), + Entry("`redhat` should violate trademark policy", "redhat", true), + Entry("`something by red hat for red hat` should violate trademark policy", "something by red hat for red hat", true), + Entry("`red hat product for red hat` should violate trademark policy", "red hat product for red hat", true), + ) +})