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

Always reflect kube-root-ca.crt configmap in offloaded namespaces #2044

Merged
merged 1 commit into from
Sep 29, 2023
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
21 changes: 21 additions & 0 deletions pkg/virtualKubelet/reflection/configuration/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,27 @@ func (ncr *NamespacedConfigMapReflector) Handle(ctx context.Context, name string
return nil
}

// ShouldSkipReflection returns whether the reflection of the given object should be skipped.
func (ncr *NamespacedConfigMapReflector) ShouldSkipReflection(obj metav1.Object) (bool, error) {
// If the object is the root CA configmap, the object must always be reflected independently
// from the reflection policy, unless the object is specifically marked with the allow or
// deny annotations.
if obj.GetName() == forge.RootCAConfigMapName {
shouldSkip, err := ncr.ForcedAllowOrSkip(obj)
switch {
case err != nil:
return false, err
case shouldSkip != nil:
return *shouldSkip, nil
default:
return false, nil
}
}

// Otherwise, the standard reflection policy is applied.
return ncr.NamespacedReflector.ShouldSkipReflection(obj)
}

// List returns the list of objects.
func (ncr *NamespacedConfigMapReflector) List() ([]interface{}, error) {
return virtualkubelet.List[virtualkubelet.Lister[*corev1.ConfigMap], *corev1.ConfigMap](
Expand Down
22 changes: 18 additions & 4 deletions pkg/virtualKubelet/reflection/configuration/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,24 @@ var _ = Describe("ConfigMap Reflection", func() {
CreateConfigMap(&local)
})

It("should succeed", func() { Expect(err).ToNot(HaveOccurred()) })
It("the remapped remote object should be created", func() {
_, err = client.CoreV1().ConfigMaps(RemoteNamespace).Get(ctx, forge.RemoteConfigMapName(name), metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
When("the reflection type is DenyList", func() {
It("should succeed", func() { Expect(err).ToNot(HaveOccurred()) })
It("the remapped remote object should be created", func() {
_, err = client.CoreV1().ConfigMaps(RemoteNamespace).Get(ctx, forge.RemoteConfigMapName(name), metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
})
})

When("the reflection type is AllowList", func() {
BeforeEach(func() {
reflectionType = consts.AllowList
})

It("should succeed", func() { Expect(err).ToNot(HaveOccurred()) })
It("the remapped remote object should be created", func() {
_, err = client.CoreV1().ConfigMaps(RemoteNamespace).Get(ctx, forge.RemoteConfigMapName(name), metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
})
})
})
})
Expand Down
35 changes: 1 addition & 34 deletions pkg/virtualKubelet/reflection/exposition/endpointslice.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"
"reflect"
"strings"
"sync"

corev1 "k8s.io/api/core/v1"
Expand All @@ -32,7 +31,6 @@ import (
corev1listers "k8s.io/client-go/listers/core/v1"
discoveryv1listers "k8s.io/client-go/listers/discovery/v1"
"k8s.io/klog/v2"
"k8s.io/utils/pointer"
"k8s.io/utils/trace"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -329,42 +327,11 @@ func (ner *NamespacedEndpointSliceReflector) UnmapEndpointIPs(ctx context.Contex
return nil
}

// forcedAllowOrSkip checks whether the given object is *explicitly* marked to be allowed or skipped
// (i.e., it has the allow or the deny annotation), independently from the reflection policy.
// If so, it returns whether the object should be skipped, or an error if unable to determine it.
// Otherwise, it return a nil bool as it is undeterminated, since we are not considering the reflection
// policy at this stage.
func forcedAllowOrSkip(obj metav1.Object) (*bool, error) {
allowAnnot, skipAnnot := false, false

value, ok := obj.GetAnnotations()[consts.AllowReflectionAnnotationKey]
if ok && !strings.EqualFold(value, "false") {
allowAnnot = true
}
value, ok = obj.GetAnnotations()[consts.SkipReflectionAnnotationKey]
if ok && !strings.EqualFold(value, "false") {
skipAnnot = true
}

if allowAnnot && skipAnnot {
return nil, fmt.Errorf("endpointslice %q can't have both the allow and deny annotations set", klog.KObj(obj))
}

switch {
case allowAnnot:
return pointer.Bool(false), nil
case skipAnnot:
return pointer.Bool(true), nil
default:
return nil, nil
}
}

// ShouldSkipReflection returns whether the reflection of the given object should be skipped.
func (ner *NamespacedEndpointSliceReflector) ShouldSkipReflection(obj metav1.Object) (bool, error) {
// Check if the endpointslice is explicitly marked to be skipped or allowed.
// If so, we do not care about the reflection policy, as the annotation takes precedence.
shouldSkip, err := forcedAllowOrSkip(obj)
shouldSkip, err := ner.ForcedAllowOrSkip(obj)
if err != nil {
return true, err
} else if shouldSkip != nil {
Expand Down
32 changes: 32 additions & 0 deletions pkg/virtualKubelet/reflection/generic/namespaced.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2"
"k8s.io/utils/pointer"

"github.com/liqotech/liqo/pkg/consts"
"github.com/liqotech/liqo/pkg/virtualKubelet/forge"
Expand Down Expand Up @@ -133,3 +134,34 @@ func (gnr *NamespacedReflector) ShouldSkipReflection(obj metav1.Object) (bool, e
func (gnr *NamespacedReflector) GetReflectionType() consts.ReflectionType {
return gnr.reflectionType
}

// ForcedAllowOrSkip checks whether the given object is *explicitly* marked to be allowed or skipped
// (i.e., it has the allow or the deny annotation), independently from the reflection policy.
// If so, it returns whether the object should be skipped, or an error if unable to determine it.
// Otherwise, it return a nil bool as it is undeterminated, since we are not considering the reflection
// policy at this stage.
func (gnr *NamespacedReflector) ForcedAllowOrSkip(obj metav1.Object) (*bool, error) {
allowAnnot, skipAnnot := false, false

value, ok := obj.GetAnnotations()[consts.AllowReflectionAnnotationKey]
if ok && !strings.EqualFold(value, "false") {
allowAnnot = true
}
value, ok = obj.GetAnnotations()[consts.SkipReflectionAnnotationKey]
if ok && !strings.EqualFold(value, "false") {
skipAnnot = true
}

if allowAnnot && skipAnnot {
return nil, fmt.Errorf("object %q can't have both the allow and deny annotations set", klog.KObj(obj))
}

switch {
case allowAnnot:
return pointer.Bool(false), nil
case skipAnnot:
return pointer.Bool(true), nil
default:
return nil, nil
}
}