Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Warn instead of panic when API resource not found or forbidden #73

Merged
merged 10 commits into from
Jul 26, 2023
29 changes: 27 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ PolicyReports CRDs. And the audit feature is disabled by default.

Then:

``` console
```console
kubectl port-forward -n kubewarden service/policy-server-default 3000:8443

./bin/audit-scanner \
Expand All @@ -16,9 +16,34 @@ kubectl port-forward -n kubewarden service/policy-server-default 3000:8443

or to get results in JSON:

``` console
```console
./bin/audit-scanner \
-k kubewarden --namespace default \
--policy-server-url https://localhost:3000 \
-l debug --print
```

### Run against audit-scanner SA

To run with the `audit-scanner` ServiceAccount, install `kubewarden-controller`
chart, and, with the help of the kubectl [view-serviceaccount-kubeconfig](https://github.com/superbrothers/kubectl-view-serviceaccount-kubeconfig-plugin)
plugin:

```console
kubectl create token audit-scanner -n kubewarden | kubectl view-serviceaccount-kubeconfig > ./kubeconfig
```

If needed, patch the resulting kubeconfig, adding the missing
`certificate-authority`. E.g:

```yaml
clusters:
- cluster:
certificate-authority: /home/vic/.minikube/ca.crt
```

And use it:

```console
export KUBECONFIG=./kubeconfig
```
2 changes: 1 addition & 1 deletion internal/policies/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (f *Fetcher) getAdmissionPolicies(namespace string) ([]policiesv1.Admission
return policies.Items, nil
}

func newClient() (client.Client, error) { //nolint
func newClient() (client.Client, error) { //nolint:ireturn
config := ctrl.GetConfigOrDie()
customScheme := scheme.Scheme
customScheme.AddKnownTypes(
Expand Down
56 changes: 52 additions & 4 deletions internal/resources/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/kubewarden/audit-scanner/internal/constants"
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
v1 "k8s.io/api/core/v1"
apimachineryerrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -59,7 +60,8 @@ func NewFetcher(kubewardenNamespace string, policyServerURL string) (*Fetcher, e
return &Fetcher{dynamicClient, kubewardenNamespace, policyServerURL, clientset}, nil
}

// GetResourcesForPolicies fetches all resources that must be audited and returns them in an AuditableResources array.
// GetResourcesForPolicies fetches all namespaced resources that must be audited
// in a specific namespace and returns them in an AuditableResources array.
// Iterates through all the rules in the policies to find all relevant resources. It creates a GVR (Group Version Resource)
// array for each rule defined in a policy. Then fetches and aggregates the GVRs for all the policies.
// Returns an array of AuditableResources. Each entry of the array will contain and array of resources of the same kind, and an array of
Expand All @@ -69,9 +71,39 @@ func (f *Fetcher) GetResourcesForPolicies(ctx context.Context, policies []polici
auditableResources := []AuditableResources{}
gvrMap := createGVRPolicyMap(policies)
for resourceFilter, policies := range gvrMap {
isNamespaced, err := f.isNamespacedResource(resourceFilter.groupVersionResource)
if err != nil {
if errors.Is(err, constants.ErrResourceNotFound) {
log.Warn().
Str("resource GVK", resourceFilter.groupVersionResource.String()).
Msg("API resource not found")
continue
}
return nil, err
}
if !isNamespaced {
// continue if resource is clusterwide
continue
}

resources, err := f.getResourcesDynamically(ctx, &resourceFilter, namespace)
// continue if resource doesn't exist.
if apimachineryerrors.IsNotFound(err) {
// continue if resource doesn't exist
log.Warn().
Dict("dict", zerolog.Dict().
Str("resource GVK", resourceFilter.groupVersionResource.String()).
Str("ns", namespace),
).Msg("API resource not found")
continue
}
if apimachineryerrors.IsForbidden(err) {
// continue if ServiceAccount lacks permissions, GVK may not exist, or
// policies may be misconfigured
log.Warn().
Dict("dict", zerolog.Dict().
Str("resource GVK", resourceFilter.groupVersionResource.String()).
Str("ns", namespace),
).Msg("API resource forbidden, unknown GVK or ServiceAccount lacks permissions")
continue
}
if err != nil {
Expand Down Expand Up @@ -119,7 +151,9 @@ func (f *Fetcher) GetClusterWideResourcesForPolicies(ctx context.Context, polici
isNamespaced, err := f.isNamespacedResource(resourceFilter.groupVersionResource)
if err != nil {
if errors.Is(err, constants.ErrResourceNotFound) {
log.Warn().Msg(fmt.Sprintf("API resource (%s) not found", resourceFilter.groupVersionResource.String()))
log.Warn().
Str("resource GVK", resourceFilter.groupVersionResource.String()).
Msg("API resource not found")
continue
}
return nil, err
Expand All @@ -128,10 +162,24 @@ func (f *Fetcher) GetClusterWideResourcesForPolicies(ctx context.Context, polici
continue
}
resources, err := f.getClusterWideResourcesDynamically(ctx, &resourceFilter)
// continue if resource doesn't exist.
if apimachineryerrors.IsNotFound(err) {
// continue if resource doesn't exist
log.Warn().
Dict("dict", zerolog.Dict().
Str("resource GVK", resourceFilter.groupVersionResource.String()),
).Msg("API resource not found")
continue
}
if apimachineryerrors.IsForbidden(err) {
// continue if ServiceAccount lacks permissions, GVK may not exist, or
// policies may be misconfigured
log.Warn().
Dict("dict", zerolog.Dict().
Str("resource GVK", resourceFilter.groupVersionResource.String()),
).Msg("API resource forbidden, unknown GVK or ServiceAccount lacks permissions")
continue
}

if err != nil {
return nil, err
}
Expand Down
161 changes: 158 additions & 3 deletions internal/resources/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,21 @@ import (
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"

apimachineryerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic/fake"

fakekubernetes "k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"
clienttesting "k8s.io/client-go/testing"
)

// policies for testing

var policy1 = policiesv1.AdmissionPolicy{
Spec: policiesv1.AdmissionPolicySpec{PolicySpec: policiesv1.PolicySpec{
Rules: []admissionregistrationv1.RuleWithOperations{{
Expand All @@ -35,6 +40,7 @@ var policy1 = policiesv1.AdmissionPolicy{
}},
}

// used to test incorrect or unknown GVKs
var policy2 = policiesv1.ClusterAdmissionPolicy{
Spec: policiesv1.ClusterAdmissionPolicySpec{PolicySpec: policiesv1.PolicySpec{
Rules: []admissionregistrationv1.RuleWithOperations{{
Expand Down Expand Up @@ -79,6 +85,36 @@ var policy4 = policiesv1.AdmissionPolicy{
}},
}

// used to test incorrect or unknown GVKs
var policyIncorrectRules = policiesv1.ClusterAdmissionPolicy{
Spec: policiesv1.ClusterAdmissionPolicySpec{PolicySpec: policiesv1.PolicySpec{
Rules: []admissionregistrationv1.RuleWithOperations{{
Operations: nil,
Rule: admissionregistrationv1.Rule{
APIGroups: []string{""},
APIVersions: []string{"v1"},
Resources: []string{"pods", "Unexistent"},
},
},
},
}},
}

// used to test skipping of clusterwide resources
var policyPodsNamespaces = policiesv1.ClusterAdmissionPolicy{
Spec: policiesv1.ClusterAdmissionPolicySpec{PolicySpec: policiesv1.PolicySpec{
Rules: []admissionregistrationv1.RuleWithOperations{{
Operations: nil,
Rule: admissionregistrationv1.Rule{
APIGroups: []string{""},
APIVersions: []string{"v1"},
Resources: []string{"pods", "namespaces"},
},
},
},
}},
}

func TestGetResourcesForPolicies(t *testing.T) {
pod1 := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -109,11 +145,37 @@ func TestGetResourcesForPolicies(t *testing.T) {
},
Spec: appsv1.DeploymentSpec{},
}
namespace1 := v1.Namespace{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "my-namespace",
},
}
customScheme := scheme.Scheme
customScheme.AddKnownTypes(policiesv1.GroupVersion, &policiesv1.ClusterAdmissionPolicy{}, &policiesv1.AdmissionPolicy{}, &policiesv1.ClusterAdmissionPolicyList{}, &policiesv1.AdmissionPolicyList{})
metav1.AddToGroupVersion(customScheme, policiesv1.GroupVersion)

dynamicClient := fake.NewSimpleDynamicClient(customScheme, &policy1, &pod1, &pod2, &pod3, &deployment1)
dynamicClient := fake.NewSimpleDynamicClient(customScheme, &policy1, &pod1, &pod2, &pod3, &deployment1, &namespace1)

apiResourceList := metav1.APIResourceList{
GroupVersion: "v1",
APIResources: []metav1.APIResource{
{
Name: "namespaces",
SingularName: "namespace",
Kind: "Namespace",
Namespaced: false,
},
{
Name: "pods",
SingularName: "pod",
Kind: "Pod",
Namespaced: true,
},
},
}
fakeClientSet := fakekubernetes.NewSimpleClientset()
fakeClientSet.Resources = []*metav1.APIResourceList{&apiResourceList}

unstructuredPod1 := map[string]interface{}{
"apiVersion": "v1",
Expand Down Expand Up @@ -155,7 +217,19 @@ func TestGetResourcesForPolicies(t *testing.T) {
Resources: []unstructured.Unstructured{{Object: unstructuredPod3}},
}}

fetcher := Fetcher{dynamicClient, "", "", nil}
expectedPIncorrectRules := []AuditableResources{{
Policies: []policiesv1.Policy{&policyIncorrectRules},
Resources: []unstructured.Unstructured{{Object: unstructuredPod1}},
// note that the resource "Unexistent" is correctly missing here
}}

expectedPPodsNamespaces := []AuditableResources{{
Policies: []policiesv1.Policy{&policyPodsNamespaces},
Resources: []unstructured.Unstructured{{Object: unstructuredPod1}},
// note that the namespacej resource is correctly missing here
}}

fetcher := Fetcher{dynamicClient, "", "", fakeClientSet}

tests := []struct {
name string
Expand All @@ -166,6 +240,8 @@ func TestGetResourcesForPolicies(t *testing.T) {
{"policy1 (just pods)", []policiesv1.Policy{&policy1}, expectedP1, "default"},
{"no policies", []policiesv1.Policy{}, []AuditableResources{}, "default"},
{"policy with label filter", []policiesv1.Policy{&policy4}, expectedP4, "kubewarden"},
{"we skip incorrect GVKs", []policiesv1.Policy{&policyIncorrectRules}, expectedPIncorrectRules, "default"},
{"we skip clusterwide resources", []policiesv1.Policy{&policyPodsNamespaces}, expectedPPodsNamespaces, "default"}, // namespaces get filtered
}

for _, test := range tests {
Expand Down Expand Up @@ -636,3 +712,82 @@ func TestIsNamespacedResource(t *testing.T) {
})
}
}

func TestLackOfPermsWhenGettingResources(t *testing.T) {
pod1 := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "podDefault",
Namespace: "default",
},
Spec: v1.PodSpec{},
}
namespace1 := v1.Namespace{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "my-namespace",
},
}
customScheme := scheme.Scheme
customScheme.AddKnownTypes(policiesv1.GroupVersion, &policiesv1.ClusterAdmissionPolicy{}, &policiesv1.AdmissionPolicy{}, &policiesv1.ClusterAdmissionPolicyList{}, &policiesv1.AdmissionPolicyList{})
metav1.AddToGroupVersion(customScheme, policiesv1.GroupVersion)

dynamicClient := fake.NewSimpleDynamicClient(customScheme, &policy1, &pod1, &namespace1)
// simulate lacking permissions when listing pods or namespaces. This should
// make the filtering skip these resources, and produce no error
dynamicClient.PrependReactor("list", "pods",
func(action clienttesting.Action) (bool, runtime.Object, error) {
return true, nil, apimachineryerrors.NewForbidden(schema.GroupResource{
Resource: "pods",
}, "", errors.New("reason"))
})
dynamicClient.PrependReactor("list", "namespaces",
func(action clienttesting.Action) (bool, runtime.Object, error) {
return true, nil, apimachineryerrors.NewForbidden(schema.GroupResource{
Resource: "namespaces",
}, "", errors.New("reason"))
})

apiResourceList := metav1.APIResourceList{
GroupVersion: "v1",
APIResources: []metav1.APIResource{
{
Name: "namespaces",
SingularName: "namespace",
Kind: "Namespace",
Namespaced: false,
},
{
Name: "pods",
SingularName: "pod",
Kind: "Pod",
Namespaced: true,
},
},
}
fakeClientSet := fakekubernetes.NewSimpleClientset()
fakeClientSet.Resources = []*metav1.APIResourceList{&apiResourceList}

// the pairs (policies,resources) should be empty, as (pods,namespaces) have
// been skipped because of lack of permissions
expectedP1 := []AuditableResources{}

fetcher := Fetcher{dynamicClient, "", "", fakeClientSet}

resources, err := fetcher.GetResourcesForPolicies(context.Background(), []policiesv1.Policy{&policy1}, "default")
if err != nil {
t.Errorf("error shouldn't have happened " + err.Error())
}
if !cmp.Equal(resources, expectedP1) {
diff := cmp.Diff(expectedP1, resources)
t.Errorf("Invalid resources: %s", diff)
}

resources, err = fetcher.GetClusterWideResourcesForPolicies(context.Background(), []policiesv1.Policy{&policy1})
if err != nil {
t.Errorf("error shouldn't have happened " + err.Error())
}
if !cmp.Equal(resources, expectedP1) {
diff := cmp.Diff(expectedP1, resources)
t.Errorf("Invalid resources: %s", diff)
}
}
Loading
Loading