Skip to content
This repository was archived by the owner on Oct 30, 2024. It is now read-only.

Test IncludeGenerated #381

Merged
merged 3 commits into from
Nov 25, 2021
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
2 changes: 1 addition & 1 deletion cmd/commands/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.15.0
0.16.0
76 changes: 64 additions & 12 deletions internal/k8sinternal/client_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package k8sinternal
package k8sinternal_test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the normal convention that the package test have a different name than the package itself? I've not done this before and I'm wondering if I should have been...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember is something to do with blackbox/whitebox testing 💭. I think when we call this a different name, we would have to import k8sinternal in order to get access to the rest of the code.

Copy link
Contributor Author

@genevieveluyt genevieveluyt Nov 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly what Brian said! It depends what you want to test.

If you use the same package as the code you're testing (whitebox), you have access to the private methods in the package and can test those.

If you use the special _test package (normally Go won't let you have multiple package names in the same directory, so appending _test to the package is a special case), you can test your code as if you were a consumer of the package (blackbox).

In this case we're not testing any private methods so using the "test" package is kind of a cheat to get out of import cycle problems. Alternatively we could move the functions that cause the import cycle into a new package and import that from both k8sinternal and test.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TYVM for the explainer!


import (
"errors"
"testing"

"github.com/Shopify/kubeaudit/internal/k8sinternal"
"github.com/Shopify/kubeaudit/internal/test"
"github.com/Shopify/kubeaudit/pkg/k8s"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
runtime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/version"
fakediscovery "k8s.io/client-go/discovery/fake"
Expand All @@ -29,11 +32,11 @@ func (kc *MockK8sClient) InClusterConfig() (*rest.Config, error) {
func TestKubeClientConfigLocal(t *testing.T) {
assert := assert.New(t)

_, err := NewKubeClientLocal("/notarealfile")
assert.Equal(ErrNoReadableKubeConfig, err)
_, err := k8sinternal.NewKubeClientLocal("/notarealfile")
assert.Equal(k8sinternal.ErrNoReadableKubeConfig, err)

_, err = NewKubeClientLocal("client.go")
assert.NotEqual(ErrNoReadableKubeConfig, err)
_, err = k8sinternal.NewKubeClientLocal("client.go")
assert.NotEqual(k8sinternal.ErrNoReadableKubeConfig, err)
assert.NotNil(err)
}

Expand All @@ -43,13 +46,13 @@ func TestKubeClientConfigCluster(t *testing.T) {
client := &MockK8sClient{}
var config *rest.Config = nil
client.On("InClusterConfig").Return(config, errors.New("mock error"))
clientset, err := NewKubeClientCluster(client)
clientset, err := k8sinternal.NewKubeClientCluster(client)
assert.Nil(clientset)
assert.NotNil(err)

client = &MockK8sClient{}
client.On("InClusterConfig").Return(&rest.Config{}, nil)
clientset, err = NewKubeClientCluster(client)
clientset, err = k8sinternal.NewKubeClientCluster(client)
assert.NotNil(clientset)
assert.NoError(err)
}
Expand All @@ -60,11 +63,11 @@ func TestIsRunningInCluster(t *testing.T) {
client := &MockK8sClient{}
var config *rest.Config = nil
client.On("InClusterConfig").Return(config, errors.New("mock error"))
assert.False(IsRunningInCluster(client))
assert.False(k8sinternal.IsRunningInCluster(client))

client = &MockK8sClient{}
client.On("InClusterConfig").Return(&rest.Config{}, nil)
assert.True(IsRunningInCluster(client))
assert.True(k8sinternal.IsRunningInCluster(client))
}

func TestGetAllResources(t *testing.T) {
Expand Down Expand Up @@ -94,13 +97,17 @@ func TestGetAllResources(t *testing.T) {
}

clientset := fakeclientset.NewSimpleClientset(resources...)
assert.Len(t, GetAllResources(clientset, ClientOptions{}), len(resourceTemplates)*len(namespaces))
assert.Len(t, k8sinternal.GetAllResources(clientset, k8sinternal.ClientOptions{}), len(resourceTemplates)*len(namespaces))

// Because field selectors are handled server-side, the fake clientset does not support them
// which means the Namespace resources don't get filtered (this is not a problem when using
// a real clientset)
// See https://github.com/kubernetes/client-go/issues/326
assert.Len(t, GetAllResources(clientset, ClientOptions{Namespace: namespaces[0]}), len(resourceTemplates)+(len(namespaces)-1))
assert.Len(
t,
k8sinternal.GetAllResources(clientset, k8sinternal.ClientOptions{Namespace: namespaces[0]}),
len(resourceTemplates)+(len(namespaces)-1),
)
}

func setNamespace(resource k8s.Resource, namespace string) {
Expand All @@ -125,7 +132,52 @@ func TestGetKubernetesVersion(t *testing.T) {
Platform: "ACME 8-bit",
}

r, err := GetKubernetesVersion(client)
r, err := k8sinternal.GetKubernetesVersion(client)
assert.Nil(t, err)
assert.EqualValues(t, *fakeDiscovery.FakedServerVersion, *r)
}

func TestIncludeGenerated(t *testing.T) {
// The "IncludeGenerated" option only applies to local and cluster mode
if !test.UseKind() {
return
}

namespace := "include-generated"
defer test.DeleteNamespace(t, namespace)
test.CreateNamespace(t, namespace)
test.ApplyManifest(t, "./fixtures/include-generated.yml", namespace)

clientset, err := k8sinternal.NewKubeClientLocal("")
require.NoError(t, err)

// Test IncludeGenerated = false
resources := k8sinternal.GetAllResources(
clientset,
k8sinternal.ClientOptions{Namespace: namespace, IncludeGenerated: false},
)
assert.False(t, hasPod(resources), "Expected no pods for IncludeGenerated=false")

// Test IncludeGenerated unspecified defaults to false
resources = k8sinternal.GetAllResources(
clientset,
k8sinternal.ClientOptions{Namespace: namespace},
)
assert.False(t, hasPod(resources), "Expected no pods if IncludeGenerated is unspecified (ie. default to false)")

// Test IncludeGenerated = true
resources = k8sinternal.GetAllResources(
clientset,
k8sinternal.ClientOptions{Namespace: namespace, IncludeGenerated: true},
)
assert.True(t, hasPod(resources), "Expected pods for IncludeGenerated=true")
}

func hasPod(resources []k8s.Resource) bool {
for _, resource := range resources {
if k8s.IsPodV1(resource) {
return true
}
}
return false
}
16 changes: 16 additions & 0 deletions internal/k8sinternal/fixtures/include-generated.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: deployment
spec:
selector:
matchLabels:
name: deployment
template:
metadata:
labels:
name: deployment
spec:
containers:
- name: container
image: scratch
4 changes: 2 additions & 2 deletions internal/test/fixtures/service.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ spec:
- protocol: TCP
port: 80
targetPort: 9376
clusterIP: 10.0.171.239
type: LoadBalancer
clusterIP: 10.96.0.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service.yml ClusterIP was invalid ("The Service "test-service" is invalid: spec.clusterIP: Invalid value: "10.0.171.239": provided IP is not in the valid range. The range of valid IPs is 10.96.0.0/16").

This doesn't affect tests or anything. I just happened to try to kubectl apply the service into a kind cluster and it failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that!

type: LoadBalancer
23 changes: 14 additions & 9 deletions internal/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func AuditLocal(t *testing.T, fixtureDir, fixture string, auditable kubeaudit.Au
}

func AuditMultiple(t *testing.T, fixtureDir, fixture string, auditables []kubeaudit.Auditable, expectedErrors []string, namespace string, mode string) {
if mode == LOCAL_MODE && os.Getenv("USE_KIND") == "false" {
if mode == LOCAL_MODE && !UseKind() {
return
}

Expand Down Expand Up @@ -104,13 +104,13 @@ func GetReport(t *testing.T, fixtureDir, fixture string, auditables []kubeaudit.
var report *kubeaudit.Report
switch mode {
case MANIFEST_MODE:
manifest, err := os.Open(fixture)
require.NoError(err)
manifest, openErr := os.Open(fixture)
require.NoError(openErr)
report, err = auditor.AuditManifest(manifest)
case LOCAL_MODE:
defer deleteNamespace(t, namespace)
createNamespace(t, namespace)
applyManifest(t, fixture, namespace)
defer DeleteNamespace(t, namespace)
CreateNamespace(t, namespace)
ApplyManifest(t, fixture, namespace)
report, err = auditor.AuditLocal("", k8sinternal.ClientOptions{Namespace: namespace})
}

Expand All @@ -133,17 +133,22 @@ func GetAllFileNames(t *testing.T, directory string) []string {
return fileNames
}

func applyManifest(t *testing.T, manifestPath, namespace string) {
// UseKind returns true if tests which utilize Kind should run
func UseKind() bool {
return os.Getenv("USE_KIND") != "false"
}

func ApplyManifest(t *testing.T, manifestPath, namespace string) {
t.Helper()
runCmd(t, exec.Command("kubectl", "apply", "-f", manifestPath, "-n", namespace))
}

func createNamespace(t *testing.T, namespace string) {
func CreateNamespace(t *testing.T, namespace string) {
t.Helper()
runCmd(t, exec.Command("kubectl", "create", "namespace", namespace))
}

func deleteNamespace(t *testing.T, namespace string) {
func DeleteNamespace(t *testing.T, namespace string) {
t.Helper()
runCmd(t, exec.Command("kubectl", "delete", "namespace", namespace))
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/k8s/type_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ func IsNamespaceV1(resource Resource) bool {
_, ok := resource.(*NamespaceV1)
return ok
}

func IsPodV1(resource Resource) bool {
_, ok := resource.(*PodV1)
return ok
}