Skip to content

Commit

Permalink
Apply Suggestions from Code Review
Browse files Browse the repository at this point in the history 
* Use a single configuration key to describe the registry as well as the
  mirror.
* More meaningful error inspection in tests.
* Use github.com/google/go-containerregistry to parse image reference.
  • Loading branch information
Sven Haardiek committed Sep 13, 2021
1 parent 034ca11 commit 5c95364
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 110 deletions.
3 changes: 1 addition & 2 deletions docs/integrations/vulnerability-scanners/trivy.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ EOF
| `trivy.serverURL` | N/A | The endpoint URL of the Trivy server. Required in `ClientServer` mode. |
| `trivy.serverTokenHeader` | `Trivy-Token` | The name of the HTTP header to send the authentication token to Trivy server. Only application in `ClientServer` mode when `trivy.serverToken` is specified. |
| `trivy.insecureRegistry.<id>` | N/A | The registry to which insecure connections are allowed. There can be multiple registries with different registry `<id>`. |
| `trivy.mirrors.registry.<id>` | N/A | A registry for which a mirror should be used to get an image. For the mirror url `trivy.mirrors.mirror.<id>` with the matching `<id>`. For each entry there must be a corresponding `trivy.mirrors.mirror.<id>` entry. There can be multiple registries with different registry `<id>`. |
| `trivy.mirrors.mirror.<id>` | N/A | The mirror for the registry with `<id>`. |
| `trivy.registry.mirror.<registry>`| N/A | Mirror for the registry `<registry>`, e.g. `trivy.registry.mirror.index.docker.io: mirror.io` would use `mirror.io` to get images originated from `index.docker.io` |
| `trivy.httpProxy` | N/A | The HTTP proxy used by Trivy to download the vulnerabilities database from GitHub. |
| `trivy.httpsProxy` | N/A | The HTTPS proxy used by Trivy to download the vulnerabilities database from GitHub. |
| `trivy.noProxy` | N/A | A comma separated list of IPs and domain names that are not subject to proxy settings. |
Expand Down
72 changes: 18 additions & 54 deletions pkg/plugin/trivy/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ const (
keyTrivyIgnoreUnfixed = "trivy.ignoreUnfixed"
keyTrivyIgnoreFile = "trivy.ignoreFile"
keyTrivyInsecureRegistryPrefix = "trivy.insecureRegistry."
keyTrivyMirrorSrcPrefix = "trivy.mirrors.registry."
keyTrivyMirrorDstPrefix = "trivy.mirrors.mirror."
keyTrivyMirrorPrefix = "trivy.registry.mirror."
keyTrivyHTTPProxy = "trivy.httpProxy"
keyTrivyHTTPSProxy = "trivy.httpsProxy"
keyTrivyNoProxy = "trivy.noProxy"
Expand Down Expand Up @@ -107,26 +106,15 @@ func (c Config) GetInsecureRegistries() map[string]bool {
return insecureRegistries
}

func (c Config) GetMirrors() (map[string]string, error) {
func (c Config) GetMirrors() map[string]string {
res := make(map[string]string)
for registyKey, registry := range c.Data {
if !strings.HasPrefix(registyKey, keyTrivyMirrorSrcPrefix) {
for registryKey, mirror := range c.Data {
if !strings.HasPrefix(registryKey, keyTrivyMirrorPrefix) {
continue
}
mirrorKey := fmt.Sprintf(
"%v%v",
keyTrivyMirrorDstPrefix,
strings.TrimPrefix(registyKey, keyTrivyMirrorSrcPrefix),
)

mirror, ok := c.Data[mirrorKey]
if !ok {
return res, fmt.Errorf("mirror %v missing for %v", mirrorKey, registyKey)
}

res[registry] = mirror
res[strings.TrimPrefix(registryKey, keyTrivyMirrorPrefix)] = mirror
}
return res, nil
return res
}

// GetResourceRequirements creates ResourceRequirements from the Config.
Expand Down Expand Up @@ -530,7 +518,7 @@ func (p *plugin) getPodSpecForStandaloneMode(config Config, spec corev1.PodSpec,
return corev1.PodSpec{}, nil, err
}

mirrors, err := config.GetMirrors()
optionalMirroredImage, err := GetMirroredImage(c.Image, config.GetMirrors())
if err != nil {
return corev1.PodSpec{}, nil, err
}
Expand All @@ -551,7 +539,7 @@ func (p *plugin) getPodSpecForStandaloneMode(config Config, spec corev1.PodSpec,
"--quiet",
"--format",
"json",
GetMirroredImage(c.Image, mirrors),
optionalMirroredImage,
},
Resources: resourceRequirements,
VolumeMounts: volumeMounts,
Expand Down Expand Up @@ -805,7 +793,7 @@ func (p *plugin) getPodSpecForClientServerMode(config Config, spec corev1.PodSpe
return corev1.PodSpec{}, nil, err
}

mirrors, err := config.GetMirrors()
optionalMirroredImage, err := GetMirroredImage(container.Image, config.GetMirrors())
if err != nil {
return corev1.PodSpec{}, nil, err
}
Expand All @@ -826,7 +814,7 @@ func (p *plugin) getPodSpecForClientServerMode(config Config, spec corev1.PodSpe
"json",
"--remote",
trivyServerURL,
GetMirroredImage(container.Image, mirrors),
optionalMirroredImage,
},
VolumeMounts: volumeMounts,
Resources: requirements,
Expand Down Expand Up @@ -981,42 +969,18 @@ func GetScoreFromCVSS(CVSSs map[string]*CVSS) *float64 {
return nvdScore
}

func fullImagePath(image string) string {
const defaultRegistry = "index.docker.io"
registryAndImage := strings.Split(image, "/")
// Empty string
if len(registryAndImage) == 0 {
return image
}

// Add default Registry, if none is present
if len(registryAndImage) == 1 ||
!(strings.Contains(registryAndImage[0], ".") ||
strings.Contains(registryAndImage[0], ":") ||
registryAndImage[0] == "localhost") {
registryAndImage = append([]string{defaultRegistry}, registryAndImage...)
}

// Add "library" if only image name is given and registry is DockerHub
if len(registryAndImage) == 2 &&
(strings.HasSuffix(registryAndImage[0], "docker.io") ||
strings.HasSuffix(registryAndImage[0], "docker.com")) {
registryAndImage = append(registryAndImage[:2], registryAndImage[1:]...)
registryAndImage[1] = "library"
func GetMirroredImage(image string, mirrors map[string]string) (string, error) {
ref, err := name.ParseReference(image)
if err != nil {
return "", err
}

return strings.Join(registryAndImage, "/")
}

func GetMirroredImage(image string, mirrors map[string]string) string {
mirroredImage := fullImagePath(image)

mirroredImage := ref.Name()
for k, v := range mirrors {
if strings.HasPrefix(mirroredImage, k) {
mirroredImage = strings.Replace(mirroredImage, k, v, 1)
return mirroredImage
return mirroredImage, nil
}
}
// If nothing is mirrord, we can simply use the input image without the fullpath.
return image
// If nothing is mirrord, we can simply use the input image.
return image, nil
}
77 changes: 23 additions & 54 deletions pkg/plugin/trivy/plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ func TestConfig_GetMirrors(t *testing.T) {
name string
configData trivy.Config
expectedOutput map[string]string
expectError bool
}{
{
name: "Should return empty map when there is no key with trivy.mirrors.registry. prefix",
Expand All @@ -281,32 +280,16 @@ func TestConfig_GetMirrors(t *testing.T) {
name: "Should return mirrors in a map",
configData: trivy.Config{PluginConfig: starboard.PluginConfig{
Data: map[string]string{
"trivy.mirrors.registry.docker": "docker.io",
"trivy.mirrors.mirror.docker": "mirror.io",
"trivy.registry.mirror.docker.io": "mirror.io",
},
}},
expectedOutput: map[string]string{"docker.io": "mirror.io"},
},
{
name: "Should return error, if mirror is missing for registry",
configData: trivy.Config{PluginConfig: starboard.PluginConfig{
Data: map[string]string{
"trivy.mirrors.registry.docker": "docker.io",
},
}},
expectError: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mirrors, err := tc.configData.GetMirrors()
if tc.expectError {
assert.Error(t, err)
} else {
assert.Equal(t, tc.expectedOutput, mirrors)
}

assert.Equal(t, tc.expectedOutput, tc.configData.GetMirrors())
})
}
}
Expand Down Expand Up @@ -1213,8 +1196,7 @@ CVE-2019-1543`,
"trivy.resources.limits.cpu": "500m",
"trivy.resources.limits.memory": "500M",

"trivy.mirrors.registry.dockerio": "index.docker.io",
"trivy.mirrors.mirror.dockerio": "mirror.io",
"trivy.registry.mirror.index.docker.io": "mirror.io",
},
workloadSpec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -2280,53 +2262,40 @@ func TestGetScoreFromCVSS(t *testing.T) {

func TestGetMirroredImage(t *testing.T) {
testCases := []struct {
name string
image string
mirrors map[string]string
expected string
name string
image string
mirrors map[string]string
expected string
expectedError string
}{
{
name: "no matching mirror, same image",
image: "index.docker.io/library/alpine",
mirrors: map[string]string{"gcr.io": "mirror.io"},
expected: "index.docker.io/library/alpine",
},
{
name: "matching mirror, changed image",
image: "index.docker.io/library/alpine",
mirrors: map[string]string{"index.docker.io": "mirror.io"},
expected: "mirror.io/library/alpine",
},
{
name: "no matching mirror, default registry",
image: "library/alpine",
name: "Mirror not match",
image: "alpine",
mirrors: map[string]string{"gcr.io": "mirror.io"},
expected: "library/alpine",
expected: "alpine",
},
{
name: "matching mirror, default registry",
image: "library/alpine",
mirrors: map[string]string{"index.docker.io": "mirror.io"},
expected: "mirror.io/library/alpine",
},
{
name: "matching mirror, default registry, expanded image name",
name: "Mirror match",
image: "alpine",
mirrors: map[string]string{"index.docker.io": "mirror.io"},
expected: "mirror.io/library/alpine",
expected: "mirror.io/library/alpine:latest",
},
{
name: "matching mirror, no expanded image name",
image: "quay.io/alpine",
mirrors: map[string]string{"quay.io": "mirror.io"},
expected: "mirror.io/alpine",
name: "Broken image",
image: "alpine@sha256:broken",
expectedError: "could not parse reference: alpine@sha256:broken",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
expected := trivy.GetMirroredImage(tc.image, tc.mirrors)
assert.Equal(t, tc.expected, expected)
expected, err := trivy.GetMirroredImage(tc.image, tc.mirrors)
if tc.expectedError != "" {
require.EqualError(t, err, tc.expectedError)
} else {
require.NoError(t, err)
assert.Equal(t, tc.expected, expected)
}
})
}
}

0 comments on commit 5c95364

Please sign in to comment.