Skip to content
Closed
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
9 changes: 6 additions & 3 deletions controller/internal/service/client/v1/client_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,10 @@ func (s *ClientService) CreateLease(ctx context.Context, req *cpb.CreateLeaseReq
return nil, err
}

if len(jlease.Spec.SharedWith) > jumpstarterdevv1alpha1.MaxSharedWithEntries {
return nil, status.Errorf(codes.InvalidArgument, "shared_with list exceeds maximum of %d entries", jumpstarterdevv1alpha1.MaxSharedWithEntries)
}
if len(jlease.Spec.SharedWith) > 0 {
// Deduplicate first, then enforce the limit against the deduplicated list,
// so a request with repeated names that collapses to a list within the
// limit is accepted rather than rejected on its raw length.
var deduped []string
for _, name := range jlease.Spec.SharedWith {
if name == jclient.Name {
Expand All @@ -345,6 +345,9 @@ func (s *ClientService) CreateLease(ctx context.Context, req *cpb.CreateLeaseReq
}
deduped = append(deduped, name)
}
if len(deduped) > jumpstarterdevv1alpha1.MaxSharedWithEntries {
return nil, status.Errorf(codes.InvalidArgument, "shared_with list exceeds maximum of %d entries", jumpstarterdevv1alpha1.MaxSharedWithEntries)
}
jlease.Spec.SharedWith = deduped
}

Expand Down
115 changes: 115 additions & 0 deletions controller/internal/service/client/v1/client_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@ package v1
import (
"context"
"testing"
"time"

jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1"
cpb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/client/v1"
"github.com/jumpstarter-dev/jumpstarter/controller/internal/service/auth"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/durationpb"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)
Expand Down Expand Up @@ -531,3 +537,112 @@ func TestApplySharedWithChanges(t *testing.T) {
}
})
}

// --- CreateLease shared_with dedup-before-limit -----------------------------

// stubContextAuthenticator satisfies authentication.ContextAuthenticator and
// always authenticates successfully with the supplied response.
type stubContextAuthenticator struct{ resp *authenticator.Response }

func (s stubContextAuthenticator) AuthenticateContext(_ context.Context) (*authenticator.Response, bool, error) {
return s.resp, true, nil
}

// stubAttributesGetter satisfies authorization.ContextAttributesGetter and
// returns fixed attributes identifying the authenticated Client object.
type stubAttributesGetter struct{ attrs authorizer.Attributes }

func (s stubAttributesGetter) ContextAttributes(_ context.Context, _ user.Info) (authorizer.Attributes, error) {
return s.attrs, nil
}

// stubAuthorizer satisfies authorizer.Authorizer and always allows.
type stubAuthorizer struct{}

func (stubAuthorizer) Authorize(_ context.Context, _ authorizer.Attributes) (authorizer.Decision, string, error) {
return authorizer.DecisionAllow, "", nil
}

// authedClientService builds a ClientService whose auth layer authenticates
// every request as owner/namespace, backed by fc for all object lookups.
func authedClientService(owner, namespace string, fc kclient.Client) *ClientService {
resp := &authenticator.Response{
User: &user.DefaultInfo{Name: "system:serviceaccount:" + namespace + ":" + owner},
}
attrs := authorizer.AttributesRecord{
User: resp.User,
Resource: "Client",
Name: owner,
Namespace: namespace,
ResourceRequest: true,
}
a := auth.NewAuth(fc, stubContextAuthenticator{resp: resp}, stubAuthorizer{}, stubAttributesGetter{attrs: attrs})
return &ClientService{Client: fc, Auth: *a}
}

func namedClients(namespace string, names ...string) []kclient.Object {
objs := make([]kclient.Object, 0, len(names))
for _, name := range names {
objs = append(objs, &jumpstarterdevv1alpha1.Client{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
})
}
return objs
}

func createLeaseReq(namespace string, sharedWith []string) *cpb.CreateLeaseRequest {
return &cpb.CreateLeaseRequest{
Parent: "namespaces/" + namespace,
Lease: &cpb.Lease{
Selector: "board=rpi4",
Duration: durationpb.New(time.Hour),
SharedWith: sharedWith,
},
}
}

func TestCreateLeaseSharedWithDedupBeforeLimit(t *testing.T) {
const ns = "default"

t.Run("raw length over limit but dedup fits is accepted", func(t *testing.T) {
// 12 raw entries, all "alice", dedup to a single entry (<= max).
objs := namedClients(ns, "owner", "alice")
svc := authedClientService("owner", ns, testFakeClient(objs...))

var shared []string
for range 12 {
shared = append(shared, "alice")
}

result, err := svc.CreateLease(context.Background(), createLeaseReq(ns, shared))
if err != nil {
t.Fatalf("expected dedup within limit to be accepted, got error: %v", err)
}
if len(result.SharedWith) != 1 || result.SharedWith[0] != "alice" {
t.Fatalf("expected deduped shared_with [alice], got %v", result.SharedWith)
}
})

t.Run("dedup still exceeding limit is rejected", func(t *testing.T) {
// 11 unique clients plus a duplicate of the first: 12 raw, 11 deduped,
// which still exceeds the maximum of 10.
var names []string
for i := range 11 {
names = append(names, "client"+string(rune('a'+i)))
}
objs := append(namedClients(ns, "owner"), namedClients(ns, names...)...)
svc := authedClientService("owner", ns, testFakeClient(objs...))

shared := append([]string{}, names...)
shared = append(shared, names[0]) // duplicate -> 12 raw / 11 deduped

_, err := svc.CreateLease(context.Background(), createLeaseReq(ns, shared))
if err == nil {
t.Fatal("expected deduped list exceeding the limit to be rejected")
}
st, ok := status.FromError(err)
if !ok || st.Code() != codes.InvalidArgument {
t.Fatalf("expected InvalidArgument, got %v", err)
}
})
}
Loading