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

Runtime: Add property for disabling caching #80245

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

aangelisc
Copy link
Contributor

@aangelisc aangelisc commented Jan 9, 2024

As a part of implementing user-based authentication, it is necessary to disable caching for security reasons. Caching should be disabled at a data source level based on the configuration of the data source.

In order to support this, an additional property has been added to the data source JSON data that will indicate to the caching service whether or not a data source instance supports caching.

Part of grafana/grafana-enterprise-mirror-shared-microsoft#170

Fixes #85634

@aangelisc aangelisc added add to changelog no-backport Skip backport of PR labels Jan 9, 2024
@aangelisc aangelisc added this to the 10.4.x milestone Jan 9, 2024
@aangelisc aangelisc self-assigned this Jan 9, 2024
@aangelisc aangelisc requested review from a team as code owners January 9, 2024 21:15
@aangelisc aangelisc requested review from tskarhed, L-M-K-B and mckn and removed request for a team January 9, 2024 21:15
@grafana-delivery-bot grafana-delivery-bot bot modified the milestones: 10.4.x, 10.3.x Jan 9, 2024
@grafana-pr-automation grafana-pr-automation bot requested review from a team and removed request for a team January 9, 2024 21:20
@grafana-pr-automation grafana-pr-automation bot requested review from a team and removed request for a team January 9, 2024 21:35
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aangelisc aangelisc merged commit a70d48a into main Jan 11, 2024
18 checks passed
@aangelisc aangelisc deleted the andreas/plugin-disable-caching branch January 11, 2024 15:57
@ifrost ifrost modified the milestones: 10.3.x, 10.4.x Jan 12, 2024
@kostrse
Copy link
Contributor

kostrse commented Jan 29, 2024

@aangelisc @mckn it's not necessary to introduce a new properly, since there's existing property oauthPassThru could be used for this purpose.

oauthPassThru is indication that authentication will be based on user's identity, so cache should not be allowed.

Cache should be disabled regardless whether it's Azure user authentication or "Forward OAuth Identity":

image

@aangelisc
Copy link
Contributor Author

@kostrse pasting my response here for brevity:

I'd rather have a distinct property which is why the new one has been added, data sources may decide to disable caching even when they have oAuthPassThru disabled so it's better to not link the two explicitly.

s0lesurviv0r pushed a commit to s0lesurviv0r/grafana that referenced this pull request Feb 3, 2024
* Add property for disabling caching

* Lint
@aangelisc aangelisc modified the milestones: 10.4.x, 10.4.0 Mar 6, 2024
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.

Plugins: Support disabling caching
6 participants