cipd: Add -tag option to 'create' and 'pkg-register' subcomands.

go/src/infra/tools/cipd/remote.go

Index: go/src/infra/tools/cipd/remote.go
diff --git a/go/src/infra/tools/cipd/remote.go b/go/src/infra/tools/cipd/remote.go
index 1904342be2b146f372ae911bafbdb4ea1f114fe7..e0ad07aa77fdc7b7dd849024efe5b0b779ab032a 100644
--- a/go/src/infra/tools/cipd/remote.go
+++ b/go/src/infra/tools/cipd/remote.go
@@ -67,6 +67,16 @@ type roleChangeMsg struct {
 	Principal string `json:"principal"`
 }
 
+// pendingProcessingError is returned by attachTags if package instance is not
+// yet ready and the call should be retried later.
+type pendingProcessingError struct {
+	message string
+}
+
+func (e *pendingProcessingError) Error() string {
+	return e.message
+}
+
 // newRemoteService is mocked in tests.
 var newRemoteService = func(client *http.Client, url string, log logging.Logger) *remoteService {
 	log.Infof("cipd: service URL is %s", url)
@@ -339,6 +349,46 @@ func (r *remoteService) modifyACL(packagePath string, changes []PackageACLChange
 	return fmt.Errorf("Unexpected reply status: %s", reply.Status)
 }
 
+func (r *remoteService) attachTags(packageName, instanceID string, tags []string) error {
+	// Tags will be passed in the request body, not via URL.
+	endpoint, err := tagsEndpoint(packageName, instanceID, nil)
+	if err != nil {
+		return err
+	}
+
+	if len(tags) == 0 {
+		return errors.New("At least one tag must be provided")
+	}
+	for _, tag := range tags {
+		err = ValidateInstanceTag(tag)
+		if err != nil {
+			return err
+		}
+	}
+	var request struct {
+		Tags []string `json:"tags"`
+	}
+	request.Tags = tags
+
+	var reply struct {
+		Status       string `json:"status"`
+		ErrorMessage string `json:"error_message"`
+	}

[nodir, 2015/05/07 04:19:02]

I like the fact that you can define a struct in Go right in the func body

+	err = r.makeRequest(endpoint, "POST", &request, &reply)
+	if err != nil {
+		return err
+	}
+	switch reply.Status {
+	case "SUCCESS":
+		return nil
+	case "PROCESSING_NOT_FINISHED_YET":
+		return &pendingProcessingError{message: reply.ErrorMessage}

[nodir, 2015/05/07 04:19:02]

nit: I think you can omit `message: ` in simple cases like this, where you
probably don't expect more fields in future

[Vadim Sh., 2015/05/07 05:07:58]

Done. I forget about this "feature".

+	case "ERROR", "PROCESSING_FAILED":
+		return errors.New(reply.ErrorMessage)
+	}
+	return fmt.Errorf("Unexpected status when attaching tags: %s", reply.Status)

[nodir, 2015/05/07 04:19:02]

I wonder if you should panic in this case. How do you decide whether you should
panic or return an error?

[Vadim Sh., 2015/05/07 05:07:58]

Simple: never panic. Once you start panicing, you'd need to use rescue to catch
panics. It introduces alternative control flow. I prefer panics to happen only
on really-really unexpected conditions and cause stack dump.

[nodir, 2015/05/07 06:47:11]

But this is a really-really unexpected condition :)

[Vadim Sh., 2015/05/07 06:50:06]

No, not really. I can totally see how server can add new error condition that
old clients don't know how to handle. Better to exit cleanly with error message
rather than die with panic.

+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 func instanceEndpoint(packageName, instanceID string) (string, error) {
@@ -366,6 +416,30 @@ func aclEndpoint(packagePath string) (string, error) {
 	return "repo/v1/acl?" + params.Encode(), nil
 }
 
+func tagsEndpoint(packageName, instanceID string, tags []string) (string, error) {
+	err := ValidatePackageName(packageName)
+	if err != nil {
+		return "", err
+	}
+	err = ValidateInstanceID(instanceID)
+	if err != nil {
+		return "", err
+	}
+	for _, tag := range tags {
+		err = ValidateInstanceTag(tag)

[nodir, 2015/05/07 04:19:02]

why would you write this code if never used?

[Vadim Sh., 2015/05/07 05:07:58]

It will be used soonish for detachTags call. I can remove it if you prefer.

+		if err != nil {
+			return "", err
+		}
+	}
+	params := url.Values{}
+	params.Add("package_name", packageName)
+	params.Add("instance_id", instanceID)
+	for _, tag := range tags {
+		params.Add("tag", tag)
+	}
+	return "repo/v1/tags?" + params.Encode(), nil
+}
+
 // convertTimestamp coverts string with int64 timestamp in microseconds since
 // to time.Time
 func convertTimestamp(ts string) (time.Time, error) {