Skip to content
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
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),
)
})
Loading