New issue
Advanced search Search tips

Issue 875064 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

CIPD not resolving instance given a known good path/version pair.

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

Issue description

I get an error when resolving the instance given a package path/version pair.

$ cipd resolve fuchsia/qemu/linux-arm64 -version version:2.10.1

Errors:
  fuchsia/qemu/linux-arm64: no such tag.


I know for a fact that an instance with this version exists.

cipd describe fuchsia/qemu/linux-arm64 -version b1b61a39e3ab0935cd030f27e01740578b04b967                                                     ✘(master) 
Package:       fuchsia/qemu/linux-arm64
Instance ID:   b1b61a39e3ab0935cd030f27e01740578b04b967
Registered by: user:cipd-builder@fuchsia-infra.iam.gserviceaccount.com
Registered at: 2017-11-30 18:35:39.34248 -0800 PST
Refs:
  latest
Tags:
  git_repository:https://fuchsia.googlesource.com/third_party/qemu
  git_revision:0f0027d617976b4cbb7b937d5836f8ef0ac25ba1
  version:2.10.1


It feels like the tag index is corrupt/buggy?
 
This is incorrect error message. The correct error message would be "
Ambiguity when resolving the tag, more than one instance has it". I'll look into it...

The problem is that version:2.10.1 is set on at least 3 different package instances:
https://chrome-infra-packages.appspot.com/p/fuchsia/qemu/linux-arm64/+/b1b61a39e3ab0935cd030f27e01740578b04b967
https://chrome-infra-packages.appspot.com/p/fuchsia/qemu/linux-arm64/+/5c0de765eec81c8587d363121a0fdac3ed17313e
https://chrome-infra-packages.appspot.com/p/fuchsia/qemu/linux-arm64/+/d1f6c82a86cc49b94b1a0e1d18c5b9a4b572d676

And CIPD can't decide which one you mean. You'll have to attach some other version tag to one of them (version:2.10.1a or something :-/).

This problem will be eventually solved (or be made less confusing) once http://go/cipd-unique-tags is implemented.
Woah, the error message is correct. Because the existing tag (the ambiguous one) is 
 actually "version:2.10.1\n": https://chrome-infra-packages.appspot.com/p/fuchsia/qemu/linux-arm64/+/version:2.10.1%0a (notice %0a at the end of the URL).

Not sure how \n crept in there. I'll add a check for tags value (currently only tag key is limited by regexp, and tag value can be arbitrary).
Cc: estaab@chromium.org
Owner: ----
Status: Available (was: Untriaged)
How urgent is this? Vadim is out for a few weeks so if we should have it sooner I can look for someone else to help out.
CIPD itself is working correctly (as in "per spec"), the spec itself is a bit too permissive (allowing '\n' in tag values).
Owner: vadimsh@chromium.org
Status: Assigned (was: Available)
I'm going to add stricter tag validation, and then sift through all existing tags and either "fix" or delete non-conforming ones.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/9dc66e204745dc8f5b8838649a9487fb381a7305

commit 9dc66e204745dc8f5b8838649a9487fb381a7305
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Oct 10 02:05:26 2018

[cipd] Add boilerplate for cipd.Admin API.

It's an RPC API accessible only to service administrators.

It will be used (at least initially) to kick off mapper jobs. Later it can grow
various adhoc methods useful when debugging issues or managing the service.

R=tandrii@chromium.org, iannucci@chromium.org, nodir@chromium.org
BUG= 875064 

Change-Id: I6eb7bc296d5adb8a226a5526e422bb14cc7a60cb
Reviewed-on: https://chromium-review.googlesource.com/c/1271923
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/api/admin/v1/admin.pb.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/api/admin/v1/admin.proto
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/api/admin/v1/adminserver_dec.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/api/admin/v1/gen.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/api/admin/v1/pb.discovery.go
[modify] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/appengine/frontend/main.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/appengine/impl/admin/acl.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/appengine/impl/admin/admin.go
[add] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/appengine/impl/admin/doc.go
[modify] https://crrev.com/9dc66e204745dc8f5b8838649a9487fb381a7305/cipd/appengine/impl/services.go

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 15

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6

commit 057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Oct 15 22:27:36 2018

[cipd] Add 'hello world' mapper.

This CL setups a framework for defining and running mappers on top of CIPD
datastore data.

R=iannucci@chromium.org, nodir@chromium.org
BUG= 875064 

Change-Id: I7756e15ff5078c677ee1ea365e547c5d948b9592
Reviewed-on: https://chromium-review.googlesource.com/c/1277970
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/api/admin/v1/admin.pb.go
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/api/admin/v1/admin.proto
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/api/admin/v1/adminserver_dec.go
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/api/admin/v1/pb.discovery.go
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/appengine/frontend/queue.yaml
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/appengine/impl/admin/acl.go
[modify] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/appengine/impl/admin/admin.go
[add] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/appengine/impl/admin/enumerate_packages.go
[add] https://crrev.com/057e98dd8b6dc2d8ab0bc1fc806b681f7a69bad6/cipd/appengine/impl/admin/mappers.go

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/5b385a870d8438e4babdd5f6a5b31fdddce63a65

commit 5b385a870d8438e4babdd5f6a5b31fdddce63a65
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Oct 16 18:02:26 2018

[cipd] Add e2e-ish test for running CIPD mappers.

Will be useful for testing more complicated mappers later.

R=iannucci@chromium.org
BUG= 875064 

Change-Id: Ie3d977b752496bc4a7eb28b1a5c66d713582c041
Reviewed-on: https://chromium-review.googlesource.com/c/1283890
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/5b385a870d8438e4babdd5f6a5b31fdddce63a65/appengine/tq/tqtesting/testable.go
[add] https://crrev.com/5b385a870d8438e4babdd5f6a5b31fdddce63a65/cipd/appengine/impl/admin/enumerate_packages_test.go
[add] https://crrev.com/5b385a870d8438e4babdd5f6a5b31fdddce63a65/cipd/appengine/impl/admin/mapper_e2e_test.go

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/8ce8205b817226e4e84bd4f4942114063a0f121f

commit 8ce8205b817226e4e84bd4f4942114063a0f121f
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Oct 16 19:31:29 2018

[cipd] Restrict allowed tag values to [A-Za-z0-9$()*+,\-./:;<=>@\\_{}~]+.

Tag values can now contain only non-whitespace printable ASCII characters except
!"#$%&?^|' (which may be misinterpreted in a CLI or URL contexts).

Most crucially this forbids \n (that somehow sneaked in into tags already), and
all non-trivial utf-8 stuff (so no more emojis in tags).

There'll be a mapper job later that will discover and fix/remove all existing
non-conformant tags.

R=iannucci@chromium.org, nodir@chromium.org
BUG= 875064 

Change-Id: I9156ebf889d6a947ddaaf221023a6b87024cdbaa
Reviewed-on: https://chromium-review.googlesource.com/c/1283892
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/8ce8205b817226e4e84bd4f4942114063a0f121f/cipd/appengine/impl/model/tag_test.go
[modify] https://crrev.com/8ce8205b817226e4e84bd4f4942114063a0f121f/cipd/common/common.go
[modify] https://crrev.com/8ce8205b817226e4e84bd4f4942114063a0f121f/cipd/common/common_test.go

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/9a068262f46bff2ad044e738854ad0129386608a

commit 9a068262f46bff2ad044e738854ad0129386608a
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Oct 16 21:00:39 2018

[cipd] Add a mapper that discovers broken tags.

Since it's not clear how many broken tags there are in the datastore, or how
costly it is to map over all existing 6M tags, prepare a mapper that just
marks broken tags. We can deal with marked tags later in an appropriate way
(depending on how many broken tags we have).

R=iannucci@chromium.org, nodir@chromium.org
BUG= 875064 

Change-Id: Id7c59f97bf36857e6a2f1d795280e4d56b8fc29e
Reviewed-on: https://chromium-review.googlesource.com/c/1282311
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/api/admin/v1/admin.pb.go
[modify] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/api/admin/v1/admin.proto
[modify] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/api/admin/v1/pb.discovery.go
[modify] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/enumerate_packages.go
[add] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/find_malformed_tags.go
[add] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/find_malformed_tags_test.go
[modify] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/mappers.go
[add] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/marked_tags.go
[add] https://crrev.com/9a068262f46bff2ad044e738854ad0129386608a/cipd/appengine/impl/admin/marked_tags_test.go

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/2697f8cc47b13d78782b6e5bcf0c8e1607102142

commit 2697f8cc47b13d78782b6e5bcf0c8e1607102142
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Oct 16 22:28:34 2018

[cipd] Allow spaces inside tag values (but not as suffix or prefix).

There are thousands of existing tags that have spaces in them (mostly
"builder:<X>" tags, where builder name have spaces).

R=iannucci@chromium.org
BUG= 875064 

Change-Id: I492e752059513a19e220e8e6046efb7019dd8f39
Reviewed-on: https://chromium-review.googlesource.com/c/1284992
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/2697f8cc47b13d78782b6e5bcf0c8e1607102142/cipd/common/common.go
[modify] https://crrev.com/2697f8cc47b13d78782b6e5bcf0c8e1607102142/cipd/common/common_test.go

Cc: phosek@chromium.org
Discovered 224 broken tags. Most of them are on Fuchsia-related packages and have \n as a suffix. 

Example packages:
fuchsia/go/mac-amd64
chromium/fuchsia/webrunner-amd64
fuchsia/qemu/linux-arm64
chromium/fuchsia/fidl

I'll run a process that "fixes" them by stripping \n. 

Also whatever script appends \n should start breaking now, since the server now refuses such tags.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/9d93d3746e86185882df18082d37142f4bd5d483

commit 9d93d3746e86185882df18082d37142f4bd5d483
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Oct 17 01:56:39 2018

[cipd] Add an admin function to fix/delete marked broken tags.

There are <300 broken tags in the datastore. It's alright to fix them in a loop.

R=iannucci@chromium.org
BUG= 875064 

Change-Id: I41762ac4b80f3ed51c8f53ee2ac1eff7c92e2808
Reviewed-on: https://chromium-review.googlesource.com/c/1285398
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/api/admin/v1/admin.pb.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/api/admin/v1/admin.proto
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/api/admin/v1/adminserver_dec.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/api/admin/v1/pb.discovery.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/appengine/impl/admin/admin.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/appengine/impl/admin/find_malformed_tags.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/appengine/impl/admin/find_malformed_tags_test.go
[modify] https://crrev.com/9d93d3746e86185882df18082d37142f4bd5d483/cipd/appengine/impl/admin/marked_tags.go

Fixed/deleted existing malformed tags (in attachment).
tags.json
43.1 KB View Download
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/d1a573e01c3823777ce96312ef03557275581d0a

commit d1a573e01c3823777ce96312ef03557275581d0a
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Oct 17 06:52:26 2018

[third_party_packages] Strip \n from firebase version string.

Otherwise it ends up in the CIPD tag where \n aren't allowed any more.

R=iannucci@chromium.org
BUG= 875064 

Change-Id: I5611942a8b6f918ef45372748d8ea2c9cf6666f3
Reviewed-on: https://chromium-review.googlesource.com/c/1286213
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#18386}
[modify] https://crrev.com/d1a573e01c3823777ce96312ef03557275581d0a/recipes/recipe_modules/third_party_packages/firebase.py

Status: Fixed (was: Assigned)

Sign in to add a comment