Skip to content

Commit

Permalink
adding method to see if a given string violates Red Hat Trademark(s) …
Browse files Browse the repository at this point in the history
…and incorporating into container checks

Signed-off-by: Adam D. Cornett <adc@redhat.com>
  • Loading branch information
acornett21 committed Dec 6, 2024
1 parent 2157229 commit ee9611b
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 14 deletions.
4 changes: 2 additions & 2 deletions container/check_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ 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() {
ctx := context.TODO()
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() {
Expand Down
4 changes: 4 additions & 0 deletions internal/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -787,13 +789,15 @@ 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{
&containerpol.HasLicenseCheck{},
containerpol.NewHasUniqueTagCheck(cfg.DockerConfig),
&containerpol.MaxLayersCheck{},
&containerpol.HasRequiredLabelsCheck{},
&containerpol.HasProhibitedContainerName{},
}, nil
}

Expand Down
4 changes: 4 additions & 0 deletions internal/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ var _ = Describe("Check Name Queries", func() {
"RunAsNonRoot",
"HasModifiedFiles",
"BasedOnUbi",
"HasProhibitedContainerName",
}),
Entry("default operator policy", OperatorPolicy, []string{
"ScorecardBasicSpecCheck",
Expand All @@ -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",
Expand All @@ -378,6 +381,7 @@ var _ = Describe("Check Name Queries", func() {
"HasRequiredLabel",
"HasModifiedFiles",
"BasedOnUbi",
"HasProhibitedContainerName",
}),
)

Expand Down
57 changes: 57 additions & 0 deletions internal/policy/container/has_prohibited_container_name.go
Original file line number Diff line number Diff line change
@@ -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.",
}
}
42 changes: 42 additions & 0 deletions internal/policy/container/has_prohibited_container_name_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
25 changes: 20 additions & 5 deletions internal/policy/container/has_required_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand All @@ -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] == "" {
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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.",
}
}
39 changes: 32 additions & 7 deletions internal/policy/container/has_required_labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,22 @@ 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",
"summary": "summary",
"description": "description",
}

if bad {
switch override {
case "remove-label":
delete(labels, "description")
case "violates-trademark":
labels["name"] = "Red Hat"
}

return labels
Expand All @@ -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
}
Expand Down Expand Up @@ -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
})
Expand All @@ -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)
Expand Down
32 changes: 32 additions & 0 deletions internal/policy/container/trademark_validator.go
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 26 additions & 0 deletions internal/policy/container/trademark_validator_test.go
Original file line number Diff line number Diff line change
@@ -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),
)
})

0 comments on commit ee9611b

Please sign in to comment.