Skip to content

Commit e15f916

Browse files
sagar-connectsagar-connect
authored
[SECENG-669] Remove 'active' attribute for policies (#750)
1 parent 830e7ff commit e15f916

File tree

6 files changed

+74
-152
lines changed

6 files changed

+74
-152
lines changed

‎api/policy/policy.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,13 @@ type httpError struct {
3030
Context map[string]interface{} `json:"context,omitempty"`
3131
}
3232

33-
// ListPolicies calls the view policy-service list policy API. If the active filter is nil, all policies are returned. If
34-
// activeFilter is not nil it will only return active or inactive policies based on the value of *activeFilter.
35-
func (c Client) ListPolicies(ownerID string, activeFilter *bool) (interface{}, error) {
33+
// ListPolicies calls the view policy-service list policy API
34+
func (c Client) ListPolicies(ownerID string) (interface{}, error) {
3635
req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/v1/owner/%s/policy", c.serverUrl, ownerID), nil)
3736
if err != nil {
3837
return nil, fmt.Errorf("failed to construct request: %v", err)
3938
}
4039

41-
query := make(url.Values)
42-
if activeFilter != nil {
43-
query.Set("active", fmt.Sprint(*activeFilter))
44-
}
45-
46-
req.URL.RawQuery = query.Encode()
47-
4840
resp, err := c.client.Do(req)
4941
if err != nil {
5042
return nil, err
@@ -54,7 +46,7 @@ func (c Client) ListPolicies(ownerID string, activeFilter *bool) (interface{}, e
5446
if resp.StatusCode != http.StatusOK {
5547
var payload httpError
5648
if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil {
57-
return nil, fmt.Errorf("unexected status-code: %d", resp.StatusCode)
49+
return nil, fmt.Errorf("unexpected status-code: %d", resp.StatusCode)
5850
}
5951
return nil, fmt.Errorf("unexpected status-code: %d - %s", resp.StatusCode, payload.Error)
6052
}
@@ -117,7 +109,6 @@ type UpdateRequest struct {
117109
Name *string `json:"name,omitempty"`
118110
Context *string `json:"context,omitempty"`
119111
Content *string `json:"content,omitempty"`
120-
Active *bool `json:"active,omitempty"`
121112
}
122113

123114
// UpdatePolicy calls the UPDATE policy API in the policy-service. It updates a policy in the policy-service matching the given owner-id and policy-id.
@@ -177,7 +168,7 @@ func (c Client) GetPolicy(ownerID string, policyID string) (interface{}, error)
177168
if resp.StatusCode != http.StatusOK {
178169
var payload httpError
179170
if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil {
180-
return nil, fmt.Errorf("unexected status-code: %d", resp.StatusCode)
171+
return nil, fmt.Errorf("unexpected status-code: %d", resp.StatusCode)
181172
}
182173
return nil, fmt.Errorf("unexpected status-code: %d - %s", resp.StatusCode, payload.Error)
183174
}
@@ -208,7 +199,7 @@ func (c Client) DeletePolicy(ownerID string, policyID string) error {
208199
if resp.StatusCode != http.StatusNoContent {
209200
var payload httpError
210201
if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil {
211-
return fmt.Errorf("unexected status-code: %d", resp.StatusCode)
202+
return fmt.Errorf("unexpected status-code: %d", resp.StatusCode)
212203
}
213204
return fmt.Errorf("unexpected status-code: %d - %s", resp.StatusCode, payload.Error)
214205
}
@@ -260,7 +251,7 @@ func (c Client) GetDecisionLogs(ownerID string, request DecisionQueryRequest) ([
260251
if resp.StatusCode != http.StatusOK {
261252
var payload httpError
262253
if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil {
263-
return nil, fmt.Errorf("unexected status-code: %d", resp.StatusCode)
254+
return nil, fmt.Errorf("unexpected status-code: %d", resp.StatusCode)
264255
}
265256
return nil, fmt.Errorf("unexpected status-code: %d - %s", resp.StatusCode, payload.Error)
266257
}

‎api/policy/policy_test.go

Lines changed: 14 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,14 @@ func TestClientListPolicies(t *testing.T) {
3636
config := &settings.Config{Token: "testtoken", HTTPClient: &http.Client{}}
3737
client := NewClient(svr.URL, config)
3838

39-
_, err := client.ListPolicies("ownerId", nil)
39+
_, err := client.ListPolicies("ownerId")
4040
assert.NilError(t, err)
4141
})
4242

43-
t.Run("List Policies - Bad Request", func(t *testing.T) {
44-
expectedResponse := `{"error": "active: query string not a boolean."}`
45-
43+
t.Run("List Policies - Forbidden", func(t *testing.T) {
44+
expectedResponse := `{"error": "Forbidden"}`
4645
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
47-
w.WriteHeader(http.StatusBadRequest)
46+
w.WriteHeader(http.StatusForbidden)
4847
_, err := w.Write([]byte(expectedResponse))
4948
assert.NilError(t, err)
5049
}))
@@ -53,13 +52,13 @@ func TestClientListPolicies(t *testing.T) {
5352
config := &settings.Config{Token: "testtoken", HTTPClient: &http.Client{}}
5453
client := NewClient(svr.URL, config)
5554

56-
policies, err := client.ListPolicies("ownerId", nil)
55+
policies, err := client.ListPolicies("ownerId")
5756
assert.Equal(t, policies, nil)
58-
assert.Error(t, err, "unexpected status-code: 400 - active: query string not a boolean.")
57+
assert.Error(t, err, "unexpected status-code: 403 - Forbidden")
5958
})
6059

61-
t.Run("List Policies - Forbidden", func(t *testing.T) {
62-
expectedResponse := `{"error": "Forbidden"}`
60+
t.Run("List Policies - Bad error json", func(t *testing.T) {
61+
expectedResponse := `{"this is bad json": }`
6362
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
6463
w.WriteHeader(http.StatusForbidden)
6564
_, err := w.Write([]byte(expectedResponse))
@@ -70,9 +69,9 @@ func TestClientListPolicies(t *testing.T) {
7069
config := &settings.Config{Token: "testtoken", HTTPClient: &http.Client{}}
7170
client := NewClient(svr.URL, config)
7271

73-
policies, err := client.ListPolicies("ownerId", nil)
72+
policies, err := client.ListPolicies("ownerId")
7473
assert.Equal(t, policies, nil)
75-
assert.Error(t, err, "unexpected status-code: 403 - Forbidden")
74+
assert.Error(t, err, "unexpected status-code: 403")
7675
})
7776

7877
t.Run("List Policies - no policies", func(t *testing.T) {
@@ -90,7 +89,7 @@ func TestClientListPolicies(t *testing.T) {
9089
config := &settings.Config{Token: "testtoken", HTTPClient: &http.Client{}}
9190
client := NewClient(svr.URL, config)
9291

93-
policies, err := client.ListPolicies("ownerId", nil)
92+
policies, err := client.ListPolicies("ownerId")
9493
assert.DeepEqual(t, policies, expectedResponseValue)
9594
assert.NilError(t, err)
9695
})
@@ -102,7 +101,6 @@ func TestClientListPolicies(t *testing.T) {
102101
"name": "policy_1",
103102
"owner_id": "462d67f8-b232-4da4-a7de-0c86dd667d3f",
104103
"context": "config",
105-
"active": false,
106104
"created_at": "2022-05-31T14:15:10.86097Z",
107105
"modified_at": null
108106
},
@@ -111,7 +109,6 @@ func TestClientListPolicies(t *testing.T) {
111109
"name": "policy_2",
112110
"owner_id": "462d67f8-b232-4da4-a7de-0c86dd667d3f",
113111
"context": "config",
114-
"active": true,
115112
"created_at": "2022-05-31T14:15:23.582383Z",
116113
"modified_at": "2022-05-31T14:15:46.72321Z"
117114
}
@@ -129,7 +126,7 @@ func TestClientListPolicies(t *testing.T) {
129126
config := &settings.Config{Token: "testtoken", HTTPClient: &http.Client{}}
130127
client := NewClient(svr.URL, config)
131128

132-
policies, err := client.ListPolicies("ownerId", nil)
129+
policies, err := client.ListPolicies("ownerId")
133130
assert.DeepEqual(t, policies, expectedResponseValue)
134131
assert.NilError(t, err)
135132
})
@@ -220,7 +217,6 @@ func TestClientGetPolicy(t *testing.T) {
220217
"owner_id": "462d67f8-b232-4da4-a7de-0c86dd667d3f",
221218
"context": "config",
222219
"content": "package test",
223-
"active": false,
224220
"created_at": "2022-05-31T14:15:10.86097Z",
225221
"modified_at": null
226222
}`
@@ -383,53 +379,13 @@ func TestClientDeletePolicy(t *testing.T) {
383379

384380
func TestClientUpdatePolicy(t *testing.T) {
385381
t.Run("expected request", func(t *testing.T) {
386-
isActive := true
387-
name := "test-name"
388-
context := "config"
389-
content := "test-content"
390-
req := UpdateRequest{
391-
Name: &name,
392-
Context: &context,
393-
Content: &content,
394-
Active: &isActive,
395-
}
396-
397-
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
398-
assert.Equal(t, r.Header.Get("circle-token"), "testtoken")
399-
assert.Equal(t, r.Header.Get("accept"), "application/json")
400-
assert.Equal(t, r.Header.Get("content-type"), "application/json")
401-
assert.Equal(t, r.Header.Get("user-agent"), version.UserAgent())
402-
assert.Equal(t, r.Header.Get("circle-token"), "testtoken")
403-
404-
assert.Equal(t, r.Method, "PATCH")
405-
assert.Equal(t, r.URL.Path, "/api/v1/owner/ownerID/policy/policyID")
406-
407-
var actual UpdateRequest
408-
assert.NilError(t, json.NewDecoder(r.Body).Decode(&actual))
409-
assert.DeepEqual(t, actual, req)
410-
411-
w.WriteHeader(http.StatusOK)
412-
_, err := w.Write([]byte("{}"))
413-
assert.NilError(t, err)
414-
}))
415-
defer svr.Close()
416-
417-
config := &settings.Config{Token: "testtoken", HTTPClient: http.DefaultClient}
418-
client := NewClient(svr.URL, config)
419-
420-
_, err := client.UpdatePolicy("ownerID", "policyID", req)
421-
assert.NilError(t, err)
422-
})
423-
424-
t.Run("nil active", func(t *testing.T) {
425382
name := "test-name"
426383
context := "config"
427384
content := "test-content"
428385
req := UpdateRequest{
429386
Name: &name,
430387
Context: &context,
431388
Content: &content,
432-
Active: nil,
433389
}
434390

435391
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -492,7 +448,7 @@ func TestClientUpdatePolicy(t *testing.T) {
492448
assert.NilError(t, json.NewDecoder(r.Body).Decode(&actual))
493449
assert.DeepEqual(t, actual, req)
494450

495-
expectedResponse := `{"error": "at least one of name, context, content, or active cannot be blank"}`
451+
expectedResponse := `{"error": "at least one of name, context, or content, cannot be blank"}`
496452
w.WriteHeader(http.StatusBadRequest)
497453
_, err := w.Write([]byte(expectedResponse))
498454
assert.NilError(t, err)
@@ -503,7 +459,7 @@ func TestClientUpdatePolicy(t *testing.T) {
503459
client := NewClient(svr.URL, config)
504460

505461
_, err := client.UpdatePolicy("ownerID", "policyID", req)
506-
assert.Error(t, err, "unexpected status-code: 400 - at least one of name, context, content, or active cannot be blank")
462+
assert.Error(t, err, "unexpected status-code: 400 - at least one of name, context, or content, cannot be blank")
507463
})
508464

509465
t.Run("one change", func(t *testing.T) {

‎cmd/policy/policy.go

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,11 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
4141
}
4242

4343
list := func() *cobra.Command {
44-
var active bool
45-
4644
cmd := &cobra.Command{
4745
Short: "List all policies",
4846
Use: "list",
4947
RunE: func(cmd *cobra.Command, args []string) error {
50-
var flags struct {
51-
Active *bool
52-
}
53-
54-
if cmd.Flag("active").Changed {
55-
flags.Active = &active
56-
}
57-
58-
policies, err := policy.NewClient(*policyBaseURL, config).ListPolicies(*ownerID, flags.Active)
48+
policies, err := policy.NewClient(*policyBaseURL, config).ListPolicies(*ownerID)
5949
if err != nil {
6050
return fmt.Errorf("failed to list policies: %v", err)
6151
}
@@ -67,11 +57,8 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
6757
return nil
6858
},
6959
Args: cobra.ExactArgs(0),
70-
Example: `policy list --owner-id 516425b2-e369-421b-838d-920e1f51b0f5 --active=true`,
60+
Example: `policy list --owner-id 516425b2-e369-421b-838d-920e1f51b0f5`,
7161
}
72-
73-
cmd.Flags().BoolVar(&active, "active", false, "(OPTIONAL) filter policies based on active status (true or false)")
74-
7562
return cmd
7663
}()
7764

@@ -164,7 +151,6 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
164151

165152
update := func() *cobra.Command {
166153
var policyPath string
167-
var active bool
168154
var context string
169155
var name string
170156
var updateRequest policy.UpdateRequest
@@ -174,10 +160,9 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
174160
Use: "update <policyID>",
175161
RunE: func(cmd *cobra.Command, args []string) error {
176162
if !(cmd.Flag("policy").Changed ||
177-
cmd.Flag("active").Changed ||
178163
cmd.Flag("context").Changed ||
179164
cmd.Flag("name").Changed) {
180-
return fmt.Errorf("one of policy, active, context, or name must be set")
165+
return fmt.Errorf("one of policy, context, or name must be set")
181166
}
182167

183168
if cmd.Flag("policy").Changed {
@@ -193,10 +178,6 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
193178

194179
client := policy.NewClient(*policyBaseURL, config)
195180

196-
if cmd.Flag("active").Changed {
197-
updateRequest.Active = &active
198-
}
199-
200181
if cmd.Flag("context").Changed {
201182
updateRequest.Context = &context
202183
}
@@ -217,12 +198,11 @@ func NewCommand(config *settings.Config, preRunE validator) *cobra.Command {
217198
return nil
218199
},
219200
Args: cobra.ExactArgs(1),
220-
Example: `policy update e9e300d1-5bab-4704-b610-addbd6e03b0b --owner-id 462d67f8-b232-4da4-a7de-0c86dd667d3f --name policy_name --active --policy ./policy.rego`,
201+
Example: `policy update e9e300d1-5bab-4704-b610-addbd6e03b0b --owner-id 462d67f8-b232-4da4-a7de-0c86dd667d3f --name policy_name --policy ./policy.rego`,
221202
}
222203

223204
cmd.Flags().StringVar(&name, "name", "", "set name of the given policy-id")
224205
cmd.Flags().StringVar(&context, "context", "", "policy context (if set, must be config)")
225-
cmd.Flags().BoolVar(&active, "active", false, "set policy active state (to deactivate, use --active=false)")
226206
cmd.Flags().StringVar(&policyPath, "policy", "", "path to rego file containing the updated policy")
227207

228208
return cmd

0 commit comments

Comments
 (0)