check_network_annotations fails with "[message] has no field named [name]" |
||
Issue description
I'm trying to land a change but can't get check_annotations.py to pass.
I'm probably just holding it wrong, but creating a bug for posterity in case others hit a similar issue.
The CL [1] adds an annotation that includes policy info:
policy {
cookies_allowed: NO
chrome_policy {
SafeSitesFilterBehavior {
SafeSitesFilterBehavior: SafeSitesFilterDisabled
}
}
setting:
"This feature is off by default. It is enabled by setting the "
"SafeSitesFilterBehavior policy to SafeSitesFilterEnabled."
}
SafeSitesFilterBehavior is a valid field in ChromeSettingsProto [2].
Yet the checker fails with a syntax error: Message type "enterprise_management.ChromeSettingsProto" has no field named "SafeSitesFilterBehavior". [3]
What am I doing wrong?
[1] https://chromium-review.googlesource.com/c/chromium/src/+/1119102
[2] https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/chrome_settings.proto?type=cs&q=ChromeSettingsProto+safesitesfilterbehavior&sq=package:chromium&g=0&l=8364
[3] https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/155869
,
Aug 17
I'll try that, but the errors have been happening on the CQ, not on my machine, and it's been a few weeks so nothing should be out of date.
,
Aug 17
The CQ error from today was indeed regarding "SafeSitesFilterDisabled", thanks for pointing that out. Maybe something was out of date earlier, but hopefully it's working now. I can't really reproduce locally, though: ./tools/traffic_annotation/scripts/check_annotations.py --build-path out_cros/rel returns immediately with no output, while ./tools/traffic_annotation/scripts/check_annotations.py --build-path out_cros/rel --complete outputs some errors but none relating to my CL (even before fixing SafeSitesFilterDisabled => 0).
,
Aug 17
This policy is quite recently added, and after each modification of the policies, the binary of traffic_annotation_auditor should be updated. We did that on Aug 7. Traffic annotation auditor does not support ChromeOS now. You should either run Windows or Linux versions. nicolaso@: Maybe it would be good to add a note somewhere (probably where a policy changer would notice) that traffic annotation auditor should also be updated.
,
Aug 17
Okay, the last CQ run failed with: (1) 'tools/traffic_annotation/summary/annotations.xml' requires update. It is recommended to run traffic_annotation_auditor locally to do the updates automatically (please refer to tools/traffic_annotation/auditor/README.md), but you can also apply the following edit(s) to do it manually: Add line: ' <item id="policy_blacklist_service" hash_code="49799644" type="0" content_hash_code="58402631" os_list="linux,mac,windows" file_path="components/policy/content/policy_blacklist_service.cc"/>' If you are using build flags that modify files (like jumbo), rerun the auditor using --all-files switch. So I guess I'll try adding that line. By the way, this is really painful. Does everyone adding a network request have to go through several failed CQ attempts, dig through READMEs and try compiling this auditor, or is it just me?
,
Aug 17
> This policy is quite recently added, and after each modification of the policies, the binary of traffic_annotation_auditor should be updated. We did that on Aug 7. Wait, the policy was added on June 26. Why is 6 weeks considered "recent"? We're talking about this CL that added the policy, right? https://chromium-review.googlesource.com/c/chromium/src/+/1109583
,
Aug 17
No, usually it's not painful! If you add a new policy, you need to update traffic_annotation_auditor binaries as well. Otherwise, if you are on Linux or Windows, you can just run traffic_annotation_auditor locally (also mentioned in the error returned by trybot) and it will either give you the errors, or automatically updates annotations.xml.
,
Aug 17
#6: Yes. Auditor does not have an automatic update cycle and is updated on demand. The infra team is working on tools to make such updates automatic and based on dependencies, but it is done now when we are notified of an error.
,
Aug 17
Thanks. Looks like everything is resolved now. |
||
►
Sign in to add a comment |
||
Comment 1 by rhalavati@chromium.org
, Aug 17