Skip to content

Commit 5ae636a

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3334 from mjudeikis/mjudeikis/add.bind.permissions.claims.flag
✨ Add --{accept/reject}-permission-claim flag to bind cli
2 parents e790143 + d39879f commit 5ae636a

File tree

3 files changed

+192
-10
lines changed

3 files changed

+192
-10
lines changed

‎cli/pkg/bind/cmd/cmd.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ var (
3030
bindExampleUses = `
3131
# Create an APIBinding named "my-binding" that binds to the APIExport "my-export" in the "root:my-service" workspace.
3232
%[1]s bind apiexport root:my-service:my-export --name my-binding
33+
34+
# Create an APIBinding named "my-binding" that binds to the APIExport "my-export" in the "root:my-service" workspace with accepted permission claims.
35+
%[1]s bind apiexport root:my-service:my-export --name my-binding --accept-permission-claims core.secrets,core.configmaps
36+
37+
# Create an APIBinding named "my-binding" that binds to the APIExport "my-export" in the "root:my-service" workspace with rejected permission claims.
38+
%[1]s bind apiexport root:my-service:my-export --name my-binding --reject-permission-claims core.secrets,core.configmaps
3339
`
3440
)
3541

‎cli/pkg/bind/plugin/bind.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"net/url"
24+
"strings"
2425
"time"
2526

2627
"github.com/spf13/cobra"
@@ -48,6 +49,15 @@ type BindOptions struct {
4849
APIBindingName string
4950
// BindWaitTimeout is how long to wait for the APIBinding to be created and successful.
5051
BindWaitTimeout time.Duration
52+
// AcceptedPermissionClaims is the list of accepted permission claims for the APIBinding.
53+
AcceptedPermissionClaims []string
54+
// RejectedPermissionClaims is the list of rejected permission claims for the APIBinding.
55+
RejectedPermissionClaims []string
56+
57+
// acceptedPermissionClaims is the parsed list of accepted permission claims for the APIBinding parsed from AcceptedPermissionClaims.
58+
acceptedPermissionClaims []apisv1alpha1.AcceptablePermissionClaim
59+
// rejectedPermissionClaims is the parsed list of rejected permission claims for the APIBinding parsed from RejectedPermissionClaims.
60+
rejectedPermissionClaims []apisv1alpha1.AcceptablePermissionClaim
5161
}
5262

5363
// NewBindOptions returns new BindOptions.
@@ -63,6 +73,8 @@ func (b *BindOptions) BindFlags(cmd *cobra.Command) {
6373

6474
cmd.Flags().StringVar(&b.APIBindingName, "name", b.APIBindingName, "Name of the APIBinding to create.")
6575
cmd.Flags().DurationVar(&b.BindWaitTimeout, "timeout", time.Second*30, "Duration to wait for APIBinding to be created successfully.")
76+
cmd.Flags().StringSliceVar(&b.AcceptedPermissionClaims, "accept-permission-claim", nil, "List of accepted permission claims for the APIBinding. Format: --accept-permission-claim resource.group")
77+
cmd.Flags().StringSliceVarP(&b.RejectedPermissionClaims, "reject-permission-claim", "", nil, "List of rejected permission claims for the APIBinding. Format: --reject-permission-claim resource.group")
6678
}
6779

6880
// Complete ensures all fields are initialized.
@@ -90,6 +102,37 @@ func (b *BindOptions) Validate() error {
90102
return fmt.Errorf("fully qualified reference to workspace where APIExport exists is required. The format is `<logical-cluster-name>:<apiexport>` or `<full>:<path>:<to>:<apiexport>`")
91103
}
92104

105+
if b.AcceptedPermissionClaims != nil {
106+
var errs []error
107+
for _, claim := range b.AcceptedPermissionClaims {
108+
if err := b.parsePermissionClaim(claim, true); err != nil {
109+
errs = append(errs, err)
110+
}
111+
}
112+
if len(errs) > 0 {
113+
return fmt.Errorf("invalid accepted permission claims: %v", errs)
114+
}
115+
}
116+
if b.RejectedPermissionClaims != nil {
117+
var errs []error
118+
for _, claim := range b.RejectedPermissionClaims {
119+
if err := b.parsePermissionClaim(claim, false); err != nil {
120+
errs = append(errs, err)
121+
}
122+
}
123+
if len(errs) > 0 {
124+
return fmt.Errorf("invalid rejected permission claims: %v", errs)
125+
}
126+
}
127+
// once parsed we can validate if the dont conflict with each other
128+
for _, acceptedClaim := range b.acceptedPermissionClaims {
129+
for _, rejectedClaim := range b.rejectedPermissionClaims {
130+
if acceptedClaim.Group == rejectedClaim.Group && acceptedClaim.Resource == rejectedClaim.Resource {
131+
return fmt.Errorf("accepted permission claim %s conflicts with rejected permission claim %s", acceptedClaim, rejectedClaim)
132+
}
133+
}
134+
}
135+
93136
return b.Options.Validate()
94137
}
95138

@@ -127,6 +170,13 @@ func (b *BindOptions) Run(ctx context.Context) error {
127170
},
128171
}
129172

173+
if len(b.acceptedPermissionClaims) > 0 {
174+
binding.Spec.PermissionClaims = b.acceptedPermissionClaims
175+
}
176+
if len(b.rejectedPermissionClaims) > 0 {
177+
binding.Spec.PermissionClaims = append(binding.Spec.PermissionClaims, b.rejectedPermissionClaims...)
178+
}
179+
130180
kcpclient, err := newKCPClusterClient(config)
131181
if err != nil {
132182
return err
@@ -164,6 +214,38 @@ func (b *BindOptions) Run(ctx context.Context) error {
164214
return nil
165215
}
166216

217+
func (b *BindOptions) parsePermissionClaim(claim string, accepted bool) error {
218+
claimParts := strings.Split(claim, ".")
219+
if len(claimParts) != 2 {
220+
return fmt.Errorf("invalid permission claim %q", claim)
221+
}
222+
223+
parsedClaim := apisv1alpha1.AcceptablePermissionClaim{}
224+
resource := claimParts[0]
225+
group := claimParts[1]
226+
if group == "core" {
227+
group = ""
228+
}
229+
230+
parsedClaim.Group = group
231+
parsedClaim.Resource = resource
232+
if accepted {
233+
parsedClaim.State = apisv1alpha1.ClaimAccepted
234+
} else {
235+
parsedClaim.State = apisv1alpha1.ClaimRejected
236+
}
237+
// TODO(mjudeikis): Once we add support for selectors/
238+
parsedClaim.All = true
239+
240+
if accepted {
241+
b.acceptedPermissionClaims = append(b.acceptedPermissionClaims, parsedClaim)
242+
} else {
243+
b.rejectedPermissionClaims = append(b.rejectedPermissionClaims, parsedClaim)
244+
}
245+
246+
return nil
247+
}
248+
167249
func newKCPClusterClient(config *rest.Config) (kcpclientset.ClusterInterface, error) {
168250
clusterConfig := rest.CopyConfig(config)
169251
u, err := url.Parse(config.Host)

‎cli/pkg/bind/plugin/bind_test.go

Lines changed: 104 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,50 +14,89 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package plugin_test
17+
package plugin
1818

1919
import (
2020
"testing"
21-
22-
"github.com/kcp-dev/kcp/cli/pkg/bind/plugin"
2321
)
2422

2523
func TestBindOptionsValidate(t *testing.T) {
2624
testCases := []struct {
2725
description string
28-
bindOptions plugin.BindOptions
26+
bindOptions BindOptions
2927
wantValid bool
3028
}{
3129
{
3230
description: "Fully-qualified APIExport reference with dashes",
33-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:test-workspace:test-apiexport-123"},
31+
bindOptions: BindOptions{APIExportRef: "test-root:test-workspace:test-apiexport-123"},
3432
wantValid: true,
3533
},
3634
{
3735
description: "Fully-qualified APIExport reference with dots in the name",
38-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:test-workspace:test.apiexport.123"},
36+
bindOptions: BindOptions{APIExportRef: "test-root:test-workspace:test.apiexport.123"},
3937
wantValid: true,
4038
},
4139
{
4240
description: "Fully-qualified APIExport reference with special characters in the name",
43-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:test-workspace:test@apiexport=123"},
41+
bindOptions: BindOptions{APIExportRef: "test-root:test-workspace:test@apiexport=123"},
4442
wantValid: true,
4543
},
4644
{
4745
description: "Fully-qualified APIExport reference with upper-case characters in the name",
48-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:test-workspace:testAPIExport123"},
46+
bindOptions: BindOptions{APIExportRef: "test-root:test-workspace:testAPIExport123"},
4947
wantValid: true,
5048
},
5149
{
5250
description: "Fully-qualified APIExport reference with special characters in the path",
53-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:t@st-workspace:test-apiexport"},
51+
bindOptions: BindOptions{APIExportRef: "test-root:t@st-workspace:test-apiexport"},
5452
wantValid: false,
5553
},
5654
{
5755
description: "Fully-qualified APIExport reference with upper-case characters in the path",
58-
bindOptions: plugin.BindOptions{APIExportRef: "test-root:TestWorkspace:test-apiexport"},
56+
bindOptions: BindOptions{APIExportRef: "test-root:TestWorkspace:test-apiexport"},
5957
wantValid: false,
6058
},
59+
{
60+
description: "Valid accepted permission claims",
61+
bindOptions: BindOptions{
62+
APIExportRef: "test-root:test-workspace:test-apiexport",
63+
AcceptedPermissionClaims: []string{"group.resource"},
64+
},
65+
wantValid: true,
66+
},
67+
{
68+
description: "Valid rejected permission claims",
69+
bindOptions: BindOptions{
70+
APIExportRef: "test-root:test-workspace:test-apiexport",
71+
RejectedPermissionClaims: []string{"resource.group"},
72+
},
73+
wantValid: true,
74+
},
75+
{
76+
description: "Conflicting accepted and rejected permission claims",
77+
bindOptions: BindOptions{
78+
APIExportRef: "test-root:test-workspace:test-apiexport",
79+
AcceptedPermissionClaims: []string{"resource.group"},
80+
RejectedPermissionClaims: []string{"resource.group"},
81+
},
82+
wantValid: false,
83+
},
84+
{
85+
description: "Invalid accepted permission claim format",
86+
bindOptions: BindOptions{
87+
APIExportRef: "test-root:test-workspace:test-apiexport",
88+
AcceptedPermissionClaims: []string{"invalidclaim"},
89+
},
90+
wantValid: false,
91+
},
92+
{
93+
description: "Invalid rejected permission claim format",
94+
bindOptions: BindOptions{
95+
APIExportRef: "test-root:test-workspace:test-apiexport",
96+
RejectedPermissionClaims: []string{"invalidclaim"},
97+
},
98+
wantValid: false,
99+
},
61100
}
62101

63102
for _, c := range testCases {
@@ -74,3 +113,58 @@ func TestBindOptionsValidate(t *testing.T) {
74113
})
75114
}
76115
}
116+
117+
func TestParsePermissionClaim(t *testing.T) {
118+
testCases := []struct {
119+
description string
120+
claim string
121+
accepted bool
122+
wantError bool
123+
}{
124+
{
125+
description: "Valid accepted permission claim",
126+
claim: "resource.group",
127+
accepted: true,
128+
wantError: false,
129+
},
130+
{
131+
description: "Valid rejected permission claim",
132+
claim: "resource.group",
133+
accepted: false,
134+
wantError: false,
135+
},
136+
{
137+
description: "Invalid permission claim format",
138+
claim: "invalidclaim",
139+
accepted: true,
140+
wantError: true,
141+
},
142+
{
143+
description: "Empty permission claim",
144+
claim: "",
145+
accepted: true,
146+
wantError: true,
147+
},
148+
{
149+
description: "Core group permission claim",
150+
claim: "resource.core",
151+
accepted: true,
152+
wantError: false,
153+
},
154+
}
155+
156+
for _, c := range testCases {
157+
t.Run(c.description, func(t *testing.T) {
158+
b := &BindOptions{}
159+
err := b.parsePermissionClaim(c.claim, c.accepted)
160+
161+
if (err != nil) && !c.wantError {
162+
t.Errorf("wanted %q to be parsed without error, but got error: %v", c.claim, err)
163+
}
164+
165+
if (err == nil) && c.wantError {
166+
t.Errorf("wanted %q to be parsed with error, but got no error", c.claim)
167+
}
168+
})
169+
}
170+
}

0 commit comments

Comments
 (0)