-
Notifications
You must be signed in to change notification settings - Fork 353
Adding Cdni oc delete for OC/CI/configuration #7432
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7432 +/- ##
============================================
- Coverage 27.12% 27.11% -0.02%
Complexity 98 98
============================================
Files 685 685
Lines 77414 77447 +33
Branches 90 90
============================================
Hits 20999 20999
- Misses 54423 54456 +33
Partials 1992 1992
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| ## [unreleased] | ||
| ### Added | ||
| - *Traffic Ops* Added cdni capabilities delete endpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you pls add the PR number and link, like in the other changelog entries?
| ]} | ||
|
|
||
| ``DELETE`` | ||
| ======= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The underline looks to be too short.
| } | ||
| defer inf.Close() | ||
|
|
||
| if inf.Config.Cdni == nil || inf.Config.Secrets[0] == "" || inf.Config.Cdni.DCdnId == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If inf.Config.Secrets has a length of 0, the second condition check will cause a panic. Could you pls add a check for that as well?
|
|
||
| :Auth. Required: Yes | ||
| :Roles Required: "admin" or "operations" | ||
| :Permissions Required: CDNI:DELETE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be CDNI-CAPACITY:DELETE as specified in the actual route?
|
I know that the other CDNi routes do not have tests, but could you please add some tests for this new endpoint? That way we can follow the same pattern for the other CDNi route tests. |
Adding a new OC endpoint compatible with current SVTA OC API testbed. DeleteConfiguration deletes CDNi configuration for ucdn specified in JWT.
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
You can use the SVTA OC API testbed, which is not open to the public but we have access to it through SVTA members, to validate that this endpoint as well other endpoints work.
PR submission checklist