Add Brave-specific options to policy templates #16351
Conversation
d3d0d15 to
c935d39
Compare
|
Removing the milestone as this still hasn't landed in |
03f21dd to
c3ed156
Compare
|
Thank you @kjozwiak. I understand better now that it doesn't have a milestone yet because it's not on |
|
(P.S.: I might be wrong on the milestone, of course. It's just what I'd think, given my limited understanding.) |
Previously, only Chromium options were there. Now we also have: * TorDisabled * IPFSEnabled * BraveRewardsDisabled * BraveWalletDisabled
c3ed156 to
9d7b4d5
Compare
|
@brave/patch-reviewers can I ask for your review please? |
| +++ b/components/policy/resources/policy_templates.json | ||
| @@ -1,4 +1,4 @@ | ||
| -{ | ||
| +__import__('policy_source_helper').BravePolicies() + { |
There was a problem hiding this comment.
will it work like this?
__import__('policy_source_helper').BravePolicies() +
{
or
__import__('policy_source_helper').BravePolicies() + \
{
There was a problem hiding this comment.
This PR will likely have to be re-done in its entirety because upstream just completely changed their implementation. Settings used to be defined in a single .json file. Now it's a set of directories and .yaml files.
| 'chrome.mac:93-', | ||
| 'chrome.linux:93-'], | ||
| 'name': | ||
| 'TorDisabled', |
There was a problem hiding this comment.
let's merge a workaround for yapf and fix this bad formatting afterwards.
#16424
|
@mherrmann this is an old one - but a good one 😄 Looking at your comment, it seems it'll have to be reworked as upstream changed how they do things. We have some additional values added since you originally did this:
https://support.brave.com/hc/en-us/articles/360039248271-Group-Policy Let me know if I can help on this one 😄 Would be cool to have something which includes these policies going forward |
|
@bsclifton yes, upstream changed things in the middle of my work. I haven't forgotten about this. I just haven't had time to get to it because I have been working on other items. You helping could mean implementing it on your own, taking this PR (what's left of it, after upstream's changes, anyways) as a starting point. |
| 'type': 'main', | ||
| 'schema': {'type': 'boolean'}, | ||
| 'name': | ||
| 'IPFSEnabled', |
There was a problem hiding this comment.
We're in the process of removing IPFS so this can go away.
|
I believe this is stale as upstream has redone things. Closing for now - we can always raise a new PR. Thanks! |
Previously, only Chromium options were there. Now we also have:
Resolves brave/brave-browser#26502.
N.B.: The Group Policy documentation should be updated after merging this issue. It currently says that the above settings are not included in the templates. The change in this PR takes effect as soon as it is merged and a Nightly release is published.
This change should only affect this file on brave-browser-downloads.s3.com. It should not have any functional effects on Brave. It's just that the template file on S3 now contains some additional options.
I'm setting the milestone to 1.46.x because this change takes effect as soon as this PR was merged and a Nightly is published.
Test plan
Check that the above settings can be now be changed via policy_templates.zip. Please see #15440 for steps that can be used to do this.
Submitter Checklist:
QA/YesorQA/No;release-notes/includeorrelease-notes/exclude;OS/...) to the associated issuenpm run test -- brave_browser_tests,npm run test -- brave_unit_testswikinpm run lint,npm run presubmitwiki,npm run gn_check,npm run tslintgit rebase master(if needed)Reviewer Checklist:
gnAfter-merge Checklist:
changes has landed on