Skip to content

Commit

Permalink
fix: disallow creation of LVMCluster outside of operator namespace an…
Browse files Browse the repository at this point in the history
…d restrict cache
  • Loading branch information
jakobmoellerdev committed Sep 20, 2023
1 parent f2d644a commit f618e3b
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 25 deletions.
108 changes: 108 additions & 0 deletions api/v1alpha1/lvmcluster_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package v1alpha1

import (
"errors"
"fmt"
"hash/fnv"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/openshift/lvm-operator/pkg/cluster"

v1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func generateUniqueNameForTestCase(ctx SpecContext) string {
GinkgoHelper()
hash := fnv.New32()
_, err := hash.Write([]byte(ctx.SpecReport().LeafNodeText))
Expect(err).ToNot(HaveOccurred())
name := fmt.Sprintf("test-%v", hash.Sum32())
By(fmt.Sprintf("Test Case %q mapped to Unique Name %q", ctx.SpecReport().LeafNodeText, name))
return name
}

var _ = Describe("webhook acceptance tests", func() {
defaultClusterTemplate := &LVMCluster{
Spec: LVMClusterSpec{
Storage: Storage{
DeviceClasses: []DeviceClass{{
Name: "test-device-class",
ThinPoolConfig: &ThinPoolConfig{
Name: "thin-pool-1",
SizePercent: 90,
OverprovisionRatio: 10,
},
Default: true,
FilesystemType: "xfs",
}},
},
},
}

It("minimum viable configuration", func(ctx SpecContext) {
generatedName := generateUniqueNameForTestCase(ctx)
GinkgoT().Setenv(cluster.OperatorNamespaceEnvVar, generatedName)
namespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: generatedName}}
Expect(k8sClient.Create(ctx, namespace)).To(Succeed())
DeferCleanup(func(ctx SpecContext) {
Expect(k8sClient.Delete(ctx, namespace)).To(Succeed())
})

resource := defaultClusterTemplate.DeepCopy()
resource.SetName(generatedName)
resource.SetNamespace(namespace.GetName())

Expect(k8sClient.Create(ctx, resource)).To(Succeed())
Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
})

It("namespace cannot be looked up via ENV", func(ctx SpecContext) {
generatedName := generateUniqueNameForTestCase(ctx)
inacceptableNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: generatedName}}
Expect(k8sClient.Create(ctx, inacceptableNamespace)).To(Succeed())
DeferCleanup(func(ctx SpecContext) {
Expect(k8sClient.Delete(ctx, inacceptableNamespace)).To(Succeed())
})

resource := defaultClusterTemplate.DeepCopy()
resource.SetName(generatedName)
resource.SetNamespace(inacceptableNamespace.GetName())

err := k8sClient.Create(ctx, resource)
Expect(err).To(HaveOccurred())
Expect(err).To(Satisfy(k8serrors.IsForbidden))

statusError := &k8serrors.StatusError{}
Expect(errors.As(err, &statusError)).To(BeTrue())
Expect(statusError.Status().Message).To(ContainSubstring(
fmt.Sprintf("%s not found", cluster.OperatorNamespaceEnvVar)))
})

It("invalid namespace gets rejected", func(ctx SpecContext) {
acceptableNamespace := "openshift-storage"
GinkgoT().Setenv(cluster.OperatorNamespaceEnvVar, acceptableNamespace)
generatedName := generateUniqueNameForTestCase(ctx)
inacceptableNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: generatedName}}
Expect(k8sClient.Create(ctx, inacceptableNamespace)).To(Succeed())
DeferCleanup(func(ctx SpecContext) {
Expect(k8sClient.Delete(ctx, inacceptableNamespace)).To(Succeed())
})

resource := defaultClusterTemplate.DeepCopy()
resource.SetName(generatedName)
resource.SetNamespace(inacceptableNamespace.GetName())

err := k8sClient.Create(ctx, resource)
Expect(err).To(HaveOccurred())
Expect(err).To(Satisfy(k8serrors.IsForbidden))

statusError := &k8serrors.StatusError{}
Expect(errors.As(err, &statusError)).To(BeTrue())
Expect(statusError.Status().Message).To(ContainSubstring(ErrInvalidNamespace.Error()))
})

})
13 changes: 12 additions & 1 deletion api/v1alpha1/lvmcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"strings"

"github.com/openshift/lvm-operator/pkg/cluster"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
Expand All @@ -35,6 +36,7 @@ var _ webhook.Validator = &LVMCluster{}
var (
ErrDeviceClassNotFound = fmt.Errorf("DeviceClass not found in the LVMCluster")
ErrThinPoolConfigNotSet = fmt.Errorf("ThinPoolConfig is not set for the DeviceClass")
ErrInvalidNamespace = fmt.Errorf("invalid namespace was supplied")
)

//+kubebuilder:webhook:path=/validate-lvm-topolvm-io-v1alpha1-lvmcluster,mutating=false,failurePolicy=fail,sideEffects=None,groups=lvm.topolvm.io,resources=lvmclusters,verbs=create;update,versions=v1alpha1,name=vlvmcluster.kb.io,admissionReviewVersions=v1
Expand All @@ -47,8 +49,17 @@ func (l *LVMCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (l *LVMCluster) ValidateCreate() (admission.Warnings, error) {
lvmclusterlog.Info("validate create", "name", l.Name)
warnings := admission.Warnings{}
lvmclusterlog.Info("validate create", "name", l.Name)

if namespace, err := cluster.GetOperatorNamespace(); err != nil {
return warnings, fmt.Errorf("could not verify namespace of lvmcluster: %w", err)
} else if namespace != l.GetNamespace() {
return warnings, fmt.Errorf(
"creating LVMCluster is only supported within namespace %q: %w",
namespace, ErrInvalidNamespace,
)
}

deviceClassWarnings, err := l.verifyDeviceClass()
warnings = append(warnings, deviceClassWarnings...)
Expand Down
13 changes: 6 additions & 7 deletions api/v1alpha1/webhook_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/client-go/kubernetes/scheme"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
//+kubebuilder:scaffold:imports
"k8s.io/apimachinery/pkg/runtime"

"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -75,23 +75,22 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
Expect(cfg).NotTo(BeNil())

scheme := runtime.NewScheme()
err = AddToScheme(scheme)
err = AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

err = admissionv1beta1.AddToScheme(scheme)
err = admissionv1beta1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:scheme

k8sClient, err = client.New(cfg, client.Options{Scheme: scheme})
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())

// start webhook server using Manager
webhookInstallOptions := &testEnv.WebhookInstallOptions
mgr, err := ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme,
Scheme: scheme.Scheme,
Metrics: metricsserver.Options{
BindAddress: "0",
},
Expand Down
24 changes: 7 additions & 17 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ package main
import (
"context"
"flag"
"fmt"
"os"

configv1 "github.com/openshift/api/config/v1"
secv1 "github.com/openshift/api/security/v1"
"github.com/openshift/lvm-operator/controllers/node"
"k8s.io/apimachinery/pkg/api/meta"
"sigs.k8s.io/controller-runtime/pkg/cache"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
Expand Down Expand Up @@ -56,9 +56,8 @@ import (
)

var (
scheme = runtime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
operatorNamespaceEnvVar = "POD_NAMESPACE"
scheme = runtime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
)

func init() {
Expand Down Expand Up @@ -90,7 +89,7 @@ func main() {
ctrl.SetLogger(logr)
klog.SetLogger(logr)

operatorNamespace, err := getOperatorNamespace()
operatorNamespace, err := cluster.GetOperatorNamespace()
if err != nil {
setupLog.Error(err, "unable to get operatorNamespace"+
"Exiting")
Expand Down Expand Up @@ -145,6 +144,9 @@ func main() {
WebhookServer: &webhook.DefaultServer{Options: webhook.Options{
Port: 9443,
}},
Cache: cache.Options{
DefaultNamespaces: map[string]cache.Config{operatorNamespace: {}},
},
HealthProbeBindAddress: probeAddr,
LeaderElectionResourceLockInterface: le.Lock,
LeaderElection: !leaderElectionConfig.Disable,
Expand Down Expand Up @@ -217,15 +219,3 @@ func main() {
os.Exit(1)
}
}

// getOperatorNamespace returns the Namespace the operator should be watching for changes
func getOperatorNamespace() (string, error) {
// The env variable POD_NAMESPACE which specifies the Namespace the pod is running in
// and hence will watch.

ns, found := os.LookupEnv(operatorNamespaceEnvVar)
if !found {
return "", fmt.Errorf("%s not found", operatorNamespaceEnvVar)
}
return ns, nil
}
20 changes: 20 additions & 0 deletions pkg/cluster/namespace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package cluster

import (
"fmt"
"os"
)

const OperatorNamespaceEnvVar = "POD_NAMESPACE"

// GetOperatorNamespace returns the Namespace the operator should be watching for changes
func GetOperatorNamespace() (string, error) {
// The env variable POD_NAMESPACE which specifies the Namespace the pod is running in
// and hence will watch.

ns, found := os.LookupEnv(OperatorNamespaceEnvVar)
if !found {
return "", fmt.Errorf("%s not found", OperatorNamespaceEnvVar)
}
return ns, nil
}

0 comments on commit f618e3b

Please sign in to comment.