Skip to content
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

Add datasource readonly feature flag and disable certain routes #7256

Closed
wants to merge 3 commits into from

Conversation

zhyuanqi
Copy link
Collaborator

@zhyuanqi zhyuanqi commented Jul 16, 2024

Description

Add datasource readonly feature flag and disable certain routes

  • /create
  • configure/opensearch

Screenshot

When datasource is enabled and readonly is false, customer should be able to create and configure opensearch.

data_source.enabled: true
data_source.readonly: false
Screen.Recording.2024-07-15.at.11.01.32.PM.mov

When datasource is enabled and readonly is true, customer should not be able to create and configure openSearch.

data_source.enabled: true
data_source.readOnly: true
Screen.Recording.2024-07-15.at.10.59.09.PM.mov

When datasource is diabled and readonly is disable, datasource section should be hidden

data_source.enabled: false
data_source.readOnly: false

Screenshot 2024-07-15 at 11 18 55 PM
Screenshot 2024-07-15 at 11 19 04 PM

When datasource is diabled and readonly is true , datasource section should still be hidden

data_source.enabled: false
data_source.readOnly: true

Screenshot 2024-07-15 at 11 20 07 PM
Screenshot 2024-07-15 at 11 20 16 PM

Testing the changes

Changelog

  • feat: Add datasource readonly feature flag and disable certain routes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Jul 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.33%. Comparing base (9893ce1) to head (a7de935).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7256      +/-   ##
==========================================
- Coverage   67.62%   67.33%   -0.30%     
==========================================
  Files        3471     3500      +29     
  Lines       68613    69224     +611     
  Branches    11165    11278     +113     
==========================================
+ Hits        46399    46610     +211     
- Misses      19510    19889     +379     
- Partials     2704     2725      +21     
Flag Coverage Δ
Linux_1 33.24% <ø> (-0.03%) ⬇️
Linux_2 55.30% <ø> (ø)
Linux_3 45.27% <100.00%> (-0.01%) ⬇️
Linux_4 ?
Windows_1 33.28% <ø> (+<0.01%) ⬆️
Windows_2 55.25% <ø> (ø)
Windows_3 45.28% <100.00%> (+<0.01%) ⬆️
Windows_4 34.71% <50.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

1 similar comment
Copy link
Contributor

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

opensearch-changeset-bot bot added a commit to zhyuanqi/OpenSearch-Dashboards that referenced this pull request Jul 16, 2024
@opensearch-project opensearch-project deleted a comment from github-actions bot Jul 16, 2024
@SuZhou-Joe
Copy link
Member

#7214 It seems we are doing the same thing on restrict user from creating data source. Could we merge the codes from both PRs?

@BionIT
Copy link
Collaborator

BionIT commented Jul 16, 2024

#7214 It seems we are doing the same thing on restrict user from creating data source. Could we merge the codes from both PRs?

Thanks @SuZhou-Joe ! Will sync offline and see if this can be combined efforts

@huyaboo
Copy link
Member

huyaboo commented Jul 16, 2024

Overall looks good although I have a question about accessing /configure/OpenSearch. Is it expected behavior to render the page if configure/OpenSearch is accessed (or /configure/* in general)?

Signed-off-by: Yuanqi(Ella) Zhu <[email protected]>
@zhyuanqi
Copy link
Collaborator Author

close this PR as will use feature flag from #7214

@zhyuanqi zhyuanqi closed this Jul 17, 2024
@zhyuanqi
Copy link
Collaborator Author

close this PR as will use feature flag from #7214

@zhyuanqi
Copy link
Collaborator Author

zhyuanqi commented Jul 18, 2024

Overall looks good although I have a question about accessing /configure/OpenSearch. Is it expected behavior to render the page if configure/OpenSearch is accessed (or /configure/* in general)?

Hi Huy. Thanks for noticing that. Yes. It is the behavior set by flint team

  <Route path={['/configure/:type']}>
              <ConfigureDirectQueryDataSourceWithRouter notifications={notifications} />
            </Route>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants