Skip to content

Commit 833314c

Browse files
krancourkrancour
authored
Merge commit from fork
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
1 parent 98b4b82 commit 833314c

7 files changed

+260
-53
lines changed

‎pkg/server/approve_freight_v1alpha1.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"connectrpc.com/connect"
1111
"github.com/gin-gonic/gin"
12-
"k8s.io/apimachinery/pkg/runtime/schema"
1312
"k8s.io/apimachinery/pkg/types"
1413
"sigs.k8s.io/controller-runtime/pkg/client"
1514

@@ -92,11 +91,7 @@ func (s *server) ApproveFreight(
9291
if err := s.authorizeFn(
9392
ctx,
9493
"promote",
95-
schema.GroupVersionResource{
96-
Group: kargoapi.GroupVersion.Group,
97-
Version: kargoapi.GroupVersion.Version,
98-
Resource: "stages",
99-
},
94+
kargoapi.GroupVersion.WithResource("stages"),
10095
"",
10196
types.NamespacedName{
10297
Namespace: project,
@@ -199,6 +194,20 @@ func (s *server) approveFreight(c *gin.Context) {
199194
return
200195
}
201196

197+
if err := s.authorizeFn(
198+
ctx,
199+
"promote",
200+
kargoapi.GroupVersion.WithResource("stages"),
201+
"",
202+
types.NamespacedName{
203+
Namespace: project,
204+
Name: stageName,
205+
},
206+
); err != nil {
207+
_ = c.Error(err)
208+
return
209+
}
210+
202211
if freight.IsApprovedFor(stageName) {
203212
c.Status(http.StatusOK)
204213
return

‎pkg/server/approve_freight_v1alpha1_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"connectrpc.com/connect"
1111
"github.com/stretchr/testify/require"
1212
corev1 "k8s.io/api/core/v1"
13+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415
"k8s.io/apimachinery/pkg/runtime/schema"
1516
"k8s.io/apimachinery/pkg/types"
@@ -441,11 +442,46 @@ func Test_server_approveFreight(t *testing.T) {
441442
require.Equal(t, http.StatusNotFound, w.Code)
442443
},
443444
},
445+
{
446+
name: "not authorized to approve (not authorized to promote)",
447+
clientBuilder: fake.NewClientBuilder().
448+
WithObjects(testProject, testFreight, testStage).
449+
WithStatusSubresource(testFreight),
450+
serverSetup: func(_ *testing.T, s *server) {
451+
s.authorizeFn = func(
452+
context.Context,
453+
string,
454+
schema.GroupVersionResource,
455+
string,
456+
client.ObjectKey,
457+
) error {
458+
return apierrors.NewForbidden(
459+
kargoapi.GroupVersion.WithResource("stages").GroupResource(),
460+
testStageName,
461+
errors.New("not authorized"),
462+
)
463+
}
464+
},
465+
assertions: func(t *testing.T, w *httptest.ResponseRecorder, _ client.Client) {
466+
require.Equal(t, http.StatusForbidden, w.Code)
467+
},
468+
},
444469
{
445470
name: "approves Freight",
446471
clientBuilder: fake.NewClientBuilder().
447472
WithObjects(testProject, testFreight, testStage).
448473
WithStatusSubresource(testFreight),
474+
serverSetup: func(_ *testing.T, s *server) {
475+
s.authorizeFn = func(
476+
context.Context,
477+
string,
478+
schema.GroupVersionResource,
479+
string,
480+
client.ObjectKey,
481+
) error {
482+
return nil
483+
}
484+
},
449485
assertions: func(t *testing.T, w *httptest.ResponseRecorder, c client.Client) {
450486
require.Equal(t, http.StatusOK, w.Code)
451487

‎pkg/server/promote_downstream_v1alpha1.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,22 @@ func (s *server) promoteDownstream(c *gin.Context) {
300300
return
301301
}
302302

303+
for _, downstream := range downstreams {
304+
if err := s.authorizeFn(
305+
ctx,
306+
"promote",
307+
kargoapi.GroupVersion.WithResource("stages"),
308+
"",
309+
types.NamespacedName{
310+
Namespace: downstream.Namespace,
311+
Name: downstream.Name,
312+
},
313+
); err != nil {
314+
_ = c.Error(err)
315+
return
316+
}
317+
}
318+
303319
// Validate that freight is available to all downstream stages
304320
for _, downstream := range downstreams {
305321
if !downstream.IsFreightAvailable(freight) {

‎pkg/server/promote_downstream_v1alpha1_test.go‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"connectrpc.com/connect"
1111
"github.com/stretchr/testify/require"
1212
corev1 "k8s.io/api/core/v1"
13+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415
"k8s.io/apimachinery/pkg/runtime/schema"
1516
"k8s.io/apimachinery/pkg/types"
@@ -791,13 +792,54 @@ func Test_server_promoteDownstream(t *testing.T) {
791792
},
792793
},
793794
{
794-
name: "Successfully promote downstream",
795+
name: "not authorized to promote to a downstream stage",
795796
clientBuilder: fake.NewClientBuilder().WithObjects(
796797
testProject,
797798
testStage,
798799
testDownstreamStage,
799800
testFreight,
800801
),
802+
serverSetup: func(_ *testing.T, s *server) {
803+
s.authorizeFn = func(
804+
context.Context,
805+
string,
806+
schema.GroupVersionResource,
807+
string,
808+
client.ObjectKey,
809+
) error {
810+
return apierrors.NewForbidden(
811+
kargoapi.GroupVersion.WithResource("stages").GroupResource(),
812+
testDownstreamStage.Name,
813+
errors.New("not authorized"),
814+
)
815+
}
816+
},
817+
body: mustJSONBody(promoteDownstreamRequest{
818+
Freight: testFreight.Name,
819+
}),
820+
assertions: func(t *testing.T, w *httptest.ResponseRecorder, _ client.Client) {
821+
require.Equal(t, http.StatusForbidden, w.Code)
822+
},
823+
},
824+
{
825+
name: "successfully promotes downstream",
826+
clientBuilder: fake.NewClientBuilder().WithObjects(
827+
testProject,
828+
testStage,
829+
testDownstreamStage,
830+
testFreight,
831+
),
832+
serverSetup: func(_ *testing.T, s *server) {
833+
s.authorizeFn = func(
834+
context.Context,
835+
string,
836+
schema.GroupVersionResource,
837+
string,
838+
client.ObjectKey,
839+
) error {
840+
return nil
841+
}
842+
},
801843
body: mustJSONBody(promoteDownstreamRequest{
802844
Freight: testFreight.Name,
803845
}),

‎pkg/server/promote_to_stage_v1alpha1.go‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"connectrpc.com/connect"
1010
"github.com/gin-gonic/gin"
1111
apierrors "k8s.io/apimachinery/pkg/api/errors"
12-
"k8s.io/apimachinery/pkg/runtime/schema"
1312
"k8s.io/apimachinery/pkg/types"
1413
"sigs.k8s.io/controller-runtime/pkg/client"
1514

@@ -94,26 +93,10 @@ func (s *server) PromoteToStage(
9493
return nil, connect.NewError(connect.CodeNotFound, err)
9594
}
9695

97-
if !s.isFreightAvailableFn(stage, freight) {
98-
// nolint:staticcheck
99-
return nil, connect.NewError(
100-
connect.CodeInvalidArgument,
101-
fmt.Errorf(
102-
"Freight %q is not available to Stage %q",
103-
freightName,
104-
stageName,
105-
),
106-
)
107-
}
108-
10996
if err = s.authorizeFn(
11097
ctx,
11198
"promote",
112-
schema.GroupVersionResource{
113-
Group: kargoapi.GroupVersion.Group,
114-
Version: kargoapi.GroupVersion.Version,
115-
Resource: "stages",
116-
},
99+
kargoapi.GroupVersion.WithResource("stages"),
117100
"",
118101
types.NamespacedName{
119102
Namespace: project,
@@ -123,6 +106,18 @@ func (s *server) PromoteToStage(
123106
return nil, err
124107
}
125108

109+
if !s.isFreightAvailableFn(stage, freight) {
110+
// nolint:staticcheck
111+
return nil, connect.NewError(
112+
connect.CodeInvalidArgument,
113+
fmt.Errorf(
114+
"Freight %q is not available to Stage %q",
115+
freightName,
116+
stageName,
117+
),
118+
)
119+
}
120+
126121
promotion, err := kargo.NewPromotionBuilder(s.client).Build(ctx, *stage, freight.Name)
127122
if err != nil {
128123
return nil, fmt.Errorf("build promotion: %w", err)
@@ -250,6 +245,20 @@ func (s *server) promoteToStage(c *gin.Context) {
250245
freight = &list.Items[0]
251246
}
252247

248+
if err := s.authorizeFn(
249+
ctx,
250+
"promote",
251+
kargoapi.GroupVersion.WithResource("stages"),
252+
"",
253+
types.NamespacedName{
254+
Namespace: project,
255+
Name: stageName,
256+
},
257+
); err != nil {
258+
_ = c.Error(err)
259+
return
260+
}
261+
253262
// Validate that the Freight is available to the Stage
254263
if !stage.IsFreightAvailable(freight) {
255264
_ = c.Error(libhttp.ErrorStr(

0 commit comments

Comments
 (0)