Skip to content

Commit

Permalink
fix(config): Implement K8s client-go in secondary API client for secr…
Browse files Browse the repository at this point in the history
…et/config retrieval
  • Loading branch information
m8rmclaren committed Dec 6, 2023
1 parent 40fefb7 commit cfa0956
Show file tree
Hide file tree
Showing 9 changed files with 330 additions and 13 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
k8s.io/api v0.26.3
k8s.io/apimachinery v0.26.3
k8s.io/client-go v0.26.3
k8s.io/klog/v2 v2.90.1
k8s.io/utils v0.0.0-20230313181309-38a27ef9d749
sigs.k8s.io/controller-runtime v0.14.6
)
Expand Down Expand Up @@ -72,7 +73,6 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiextensions-apiserver v0.26.3 // indirect
k8s.io/component-base v0.26.3 // indirect
k8s.io/klog/v2 v2.90.1 // indirect
k8s.io/kube-aggregator v0.26.3 // indirect
k8s.io/kube-openapi v0.0.0-20230327201221-f5883ff37f0c // indirect
sigs.k8s.io/gateway-api v0.6.2 // indirect
Expand Down
12 changes: 8 additions & 4 deletions internal/controllers/certificaterequest_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ var (

type CertificateRequestReconciler struct {
client.Client
ConfigClient issuerutil.ConfigClient
Scheme *runtime.Scheme
SignerBuilder signer.CommandSignerBuilder
ClusterResourceNamespace string
Expand Down Expand Up @@ -156,8 +157,8 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R

// Add a Ready condition if one does not already exist
if ready := cmutil.GetCertificateRequestCondition(&certificateRequest, cmapi.CertificateRequestConditionReady); ready == nil {
log.Info("Initialising Ready condition")
setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Initialising")
log.Info("Initializing Ready condition")
setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonPending, "Initializing")
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -214,13 +215,16 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
return ctrl.Result{}, errIssuerNotReady
}

// Set the context on the config client
r.ConfigClient.SetContext(ctx)

authSecretName := types.NamespacedName{
Name: issuerSpec.SecretName,
Namespace: secretNamespace,
}

var authSecret corev1.Secret
if err := r.Get(ctx, authSecretName, &authSecret); err != nil {
if err = r.ConfigClient.GetSecret(authSecretName, &authSecret); err != nil {
return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetAuthSecret, authSecretName, err)
}

Expand All @@ -233,7 +237,7 @@ func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.R
var caSecret corev1.Secret
if issuerSpec.CaSecretName != "" {
// If the CA secret name is not specified, we will not attempt to retrieve it
err = r.Get(ctx, caSecretName, &caSecret)
err = r.ConfigClient.GetSecret(caSecretName, &caSecret)
if err != nil {
return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetCaSecret, caSecretName, err)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/controllers/certificaterequest_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
cmgen "github.com/cert-manager/cert-manager/test/unit/gen"
logrtesting "github.com/go-logr/logr/testing"
logrtesting "github.com/go-logr/logr/testr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -603,6 +603,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
Build()
controller := CertificateRequestReconciler{
Client: fakeClient,
ConfigClient: NewFakeConfigClient(fakeClient),
Scheme: scheme,
ClusterResourceNamespace: tc.clusterResourceNamespace,
SignerBuilder: tc.Builder,
Expand All @@ -611,7 +612,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
SecretAccessGrantedAtClusterLevel: true,
}
result, err := controller.Reconcile(
ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)),
ctrl.LoggerInto(context.TODO(), logrtesting.New(t)),
reconcile.Request{NamespacedName: tc.name},
)
if tc.expectedError != nil {
Expand Down
57 changes: 57 additions & 0 deletions internal/controllers/fake_configclient_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
Copyright 2023 The Keyfactor Command Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers

import (
"context"
"github.com/Keyfactor/command-issuer/internal/issuer/util"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// FakeConfigClient is a fake implementation of the util.ConfigClient interface
// It forwards requests destined for the Kubernetes API server implemented by
// the util.ConfigClient interface to a fake Kubernetes API server implemented
// by the client.Client interface.

// Force the compiler to check that FakeConfigClient implements the util.ConfigClient interface
var _ util.ConfigClient = &FakeConfigClient{}

type FakeConfigClient struct {
client client.Client
ctx context.Context
}

// NewFakeConfigClient uses the
func NewFakeConfigClient(fakeControllerRuntimeClient client.Client) util.ConfigClient {
return &FakeConfigClient{
client: fakeControllerRuntimeClient,
}
}

func (f FakeConfigClient) SetContext(ctx context.Context) {
f.ctx = ctx
}

func (f FakeConfigClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error {
return f.client.Get(f.ctx, name, out)
}

func (f FakeConfigClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error {
return f.client.Get(f.ctx, name, out)
}
10 changes: 7 additions & 3 deletions internal/controllers/issuer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ var (
// IssuerReconciler reconciles a Issuer object
type IssuerReconciler struct {
client.Client
ConfigClient issuerutil.ConfigClient
Kind string
ClusterResourceNamespace string
SecretAccessGrantedAtClusterLevel bool
Expand All @@ -73,7 +74,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res

issuer, err := r.newIssuer()
if err != nil {
log.Error(err, "Unrecognised issuer type")
log.Error(err, "Unrecognized issuer type")
return ctrl.Result{}, nil
}
if err := r.Get(ctx, req.NamespacedName, issuer); err != nil {
Expand Down Expand Up @@ -125,8 +126,11 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
authSecretName.Namespace = r.ClusterResourceNamespace
}

// Set the context on the config client
r.ConfigClient.SetContext(ctx)

var authSecret corev1.Secret
if err := r.Get(ctx, authSecretName, &authSecret); err != nil {
if err := r.ConfigClient.GetSecret(authSecretName, &authSecret); err != nil {
return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetAuthSecret, authSecretName, err)
}

Expand All @@ -139,7 +143,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
var caSecret corev1.Secret
if issuerSpec.CaSecretName != "" {
// If the CA secret name is not specified, we will not attempt to retrieve it
err = r.Get(ctx, caSecretName, &caSecret)
err = r.ConfigClient.GetSecret(caSecretName, &caSecret)
if err != nil {
return ctrl.Result{}, fmt.Errorf("%w, secret name: %s, reason: %v", errGetCaSecret, caSecretName, err)
}
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/issuer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"errors"
"github.com/Keyfactor/command-issuer/internal/issuer/signer"
issuerutil "github.com/Keyfactor/command-issuer/internal/issuer/util"
logrtesting "github.com/go-logr/logr/testing"
logrtesting "github.com/go-logr/logr/testr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -126,7 +126,7 @@ func TestIssuerReconcile(t *testing.T) {
expectedReadyConditionStatus: commandissuer.ConditionTrue,
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
},
"issuer-kind-unrecognised": {
"issuer-kind-Unrecognized": {
kind: "UnrecognizedType",
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
},
Expand Down Expand Up @@ -253,13 +253,14 @@ func TestIssuerReconcile(t *testing.T) {
controller := IssuerReconciler{
Kind: tc.kind,
Client: fakeClient,
ConfigClient: NewFakeConfigClient(fakeClient),
Scheme: scheme,
HealthCheckerBuilder: tc.healthCheckerBuilder,
ClusterResourceNamespace: tc.clusterResourceNamespace,
SecretAccessGrantedAtClusterLevel: true,
}
result, err := controller.Reconcile(
ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)),
ctrl.LoggerInto(context.TODO(), logrtesting.New(t)),
reconcile.Request{NamespacedName: tc.name},
)
if tc.expectedError != nil {
Expand Down
152 changes: 152 additions & 0 deletions internal/issuer/util/configclient.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
Copyright 2023 The Keyfactor Command Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package util

import (
"context"
"fmt"
authv1 "k8s.io/api/authorization/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
)

type ConfigClient interface {
SetContext(ctx context.Context)
GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error
GetSecret(name types.NamespacedName, out *corev1.Secret) error
}

type configClient struct {
ctx context.Context
logger klog.Logger
client kubernetes.Interface
accessCache map[string]bool

verifyAccessFunc func(apiResource string, resource types.NamespacedName) error
}

func NewConfigClient(ctx context.Context) (ConfigClient, error) {
config := ctrl.GetConfigOrDie()

// Create the clientset
clientset, err := kubernetes.NewForConfig(config)
if err != nil {
return nil, fmt.Errorf("failed to create clientset: %w", err)
}

client := &configClient{
client: clientset,
accessCache: make(map[string]bool),
ctx: ctx,
logger: klog.NewKlogr(),
}

client.verifyAccessFunc = client.verifyAccessToResource

return client, nil
}

func (c *configClient) SetContext(ctx context.Context) {
c.ctx = ctx
c.logger = klog.FromContext(ctx)
}

func (c *configClient) verifyAccessToResource(apiResource string, resource types.NamespacedName) error {
verbs := []string{"get", "list", "watch"}

for _, verb := range verbs {
ssar := &authv1.SelfSubjectAccessReview{
Spec: authv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authv1.ResourceAttributes{
Name: resource.Name,
Namespace: resource.Namespace,

Group: "",
Resource: apiResource,
Verb: verb,
},
},
}

ssar, err := c.client.AuthorizationV1().SelfSubjectAccessReviews().Create(c.ctx, ssar, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("failed to create SelfSubjectAccessReview to check access to %s for verb %q: %w", apiResource, verb, err)
}

if !ssar.Status.Allowed {
return fmt.Errorf("client does not have access to %s called %q for verb %q, reason: %v", apiResource, resource.String(), verb, ssar.Status.String())
}
}

c.logger.Info(fmt.Sprintf("Client has access to %s called %q", apiResource, resource.String()))

return nil
}

func (c *configClient) GetConfigMap(name types.NamespacedName, out *corev1.ConfigMap) error {
if c == nil {
return fmt.Errorf("config client is nil")
}

// Check if the client has access to the configmap resource
if ok, _ := c.accessCache[name.String()]; !ok {
err := c.verifyAccessFunc("configmaps", name)
if err != nil {
return err
}
c.accessCache[name.String()] = true
}

// Get the configmap
configmap, err := c.client.CoreV1().ConfigMaps(name.Namespace).Get(c.ctx, name.Name, metav1.GetOptions{})
if err != nil {
return err
}

// Copy the configmap into the out parameter
configmap.DeepCopyInto(out)
return nil
}

func (c *configClient) GetSecret(name types.NamespacedName, out *corev1.Secret) error {
if c == nil {
return fmt.Errorf("config client is nil")
}

// Check if the client has access to the secret resource
if ok, _ := c.accessCache[name.String()]; !ok {
err := c.verifyAccessFunc("secrets", name)
if err != nil {
return err
}
c.accessCache[name.String()] = true
}

// Get the secret
secret, err := c.client.CoreV1().Secrets(name.Namespace).Get(c.ctx, name.Name, metav1.GetOptions{})
if err != nil {
return err
}

// Copy the secret into the out parameter
secret.DeepCopyInto(out)
return nil
}
Loading

0 comments on commit cfa0956

Please sign in to comment.