-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds test for APIs related to security plugin and updates spec to add new APIs. #439
Adds test for APIs related to security plugin and updates spec to add new APIs. #439
Conversation
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Changes AnalysisCommit SHA: 705e6cf API ChangesSummary
ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10356910213/artifacts/1803095089 API Coverage
|
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Is "with API prefix" recommended over "without API prefix" or vice-versa? Are they the same thing If one is superseded by another then we should move it, mark it as deprecated and likely not test it twice. |
These APIs are completely different: The difference is |
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
…s added by linter Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
8c8c1fe
to
70e9f4c
Compare
spec/namespaces/security.yaml
Outdated
name: timeout | ||
in: query | ||
schema: | ||
type: string |
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.
Are these strings or one of the duration types?
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.
it is expected as a string to be utilized here: https://github.com/opensearch-project/security/blob/cc26b94ca84b562ae75ed8e2958a343831682042/src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java#L89
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.
I mean should this be
schema:
$ref: '../schemas/_common.yaml#/components/schemas/Duration'
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.
i see, I have replaced it.
Signed-off-by: Darshit Chanpura <[email protected]>
…ecurity-tests Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
.github/workflows/test-spec.yml
Outdated
tests: plugins/security | ||
- version: 2.0.0 | ||
admin_password: admin | ||
tests: plugins/security |
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.
Are these APIs different in 1.3.x, 2.0 and 2.16? If they aren't very different only leave the latest, 2.16.
payload: | ||
- op: replace | ||
path: /config/dynamic/authc/basic_internal_auth_domain/transport_enabled | ||
value: 'true' |
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.
This is not a bool?
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.
accepts either or. will change it to bool
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Once 2.17 and 3.0 tags on opensearchstaging will be updated, the ci will pass for those two versions. |
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
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.
@dblock Would you please re-review this?
Signed-off-by: Darshit Chanpura <[email protected]>
c1f99b5
to
7e6bf0c
Compare
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.
Needs some file naming consistency in tests.
tests/default/security/api/action_groups.yaml -> tests/default/security/api/actiongroups.yaml
etc.
Thanks!
Signed-off-by: Darshit Chanpura <[email protected]>
@dblock I've renamed everything to match API path. Could you please re-run CI as the test-failure is not related to security plugin? |
@DarshitChanpura in #510 I marked a lot of the security API tests to be skipped when running against Amazon Managed / Serverless OpenSearch. You might want to check that it's correct. I found some discrepancies in |
Description
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.