New issue
Advanced search Search tips

Issue 875106 link

Starred by 1 user

Issue metadata

Status: Verified
Owner: ----
Closed: Aug 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

check_network_annotations fails with "[message] has no field named [name]"

Project Member Reported by michae...@chromium.org, Aug 16

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
 
Cc: nicolaso@chromium.org
I cannot reproduce the error on my machine. The most likely reason that I think of is your copy of traffic_annotation_auditor might be out dated. Have you run 'gclient sync' in the last 10 days?

Also, you need to change "SafeSitesFilterBehavior: SafeSitesFilterDisabled" to "SafeSitesFilterBehavior: 0".
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.
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).
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.
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?
> 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
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.
#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.
Status: Verified (was: Untriaged)
Thanks. Looks like everything is resolved now.

Sign in to add a comment