Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gitserver: protobuf: relax ArchiveRequest path fields to be bytes instead of string #61970

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

ggilmore
Copy link
Contributor

@ggilmore ggilmore commented Apr 17, 2024

On customer instances and on sourcegraph.com, we get the following errors like the following from the gRPC internal error reporter:

https://cloudlogging.app.goo.gl/mLTBtcFd7w6SgBii9

{
  "insertId": "94nv6f3zi8abqz6s",
  "jsonPayload": {
    "message": "grpc: error while marshaling: string field contains invalid UTF-8",
    "Resource": {
      "service.version": "269484_2024-04-17_5.3-780a5114fefa",
      "service.instance.id": "symbols-0",
      "service.name": "symbols"
    },
    "Function": "github.com/sourcegraph/sourcegraph/internal/grpc/internalerrs.doLog",
    "Attributes": {
      "grpcCode": "Internal",
      "grpcService": "gitserver.v1.GitserverService",
      "nonUTF8StringFields": [
        "paths[5]"
      ],
      "grpcMethod": "Archive"
    },
    "Caller": "internalerrs/logging.go:239",
    "timestampNanos": 1713348262741927000,
    "InstrumentationScope": "gitserver.client.gRPC.internal.error.reporter.streamingMethod.postMessageSend"
  },
  "resource": {
    "type": "k8s_container",
    "labels": {
      "project_id": "src-747bc765eb31a4873e4b",
      "cluster_name": "src-1b4ce535b4573846",
      "container_name": "symbols",
      "location": "us-central1",
      "pod_name": "symbols-0",
      "namespace_name": "src-f7c368f6f7282eff920c"
    }
  },
  "timestamp": "2024-04-17T10:04:22.742097418Z",
  "severity": "ERROR",
  "labels": {
    "k8s-pod/app_kubernetes_io/component": "symbols",
    "k8s-pod/controller-revision-hash": "symbols-66478df6f7",
    "k8s-pod/app_kubernetes_io/instance": "src-bd02273f6b90d1d1beee",
    "k8s-pod/app": "symbols",
    "k8s-pod/deploy": "sourcegraph",
    "k8s-pod/app_kubernetes_io/name": "sourcegraph",
    "compute.googleapis.com/resource_name": "gke-src-1b4ce535b457-nap-n2-highmem-8-7a033d25-4tbj",
    "k8s-pod/statefulset_kubernetes_io/pod-name": "symbols-0"
  },
  "logName": "projects/src-747bc765eb31a4873e4b/logs/stderr",
  "receiveTimestamp": "2024-04-17T10:04:25.081206904Z"
}

This error comes from the gitserver gRPC clients when they tries to send an ArchiveRequest

// ArchiveRequest is set of parameters for the Archive RPC.
message ArchiveRequest {
// repo is the name of the repo to be archived.
string repo = 1;
// treeish is the tree or commit to produce an archive for.
string treeish = 2;
// format is the format of the resulting archive (either ZIP or TAR).
ArchiveFormat format = 3;
// paths is the list of paths to include in the archive. If empty, all
// paths are included.
repeated string paths = 4;
}

In protobuf, all string fields must be utf-8 encoded. However, the UNIX specification says that filepaths can contain any byte sequence without NULL or /, which includes non-utf8 encoded strings.

So, in order to pass this information along correctly - we need to change the type for paths in ArchiveRequest from string to bytes - which allows for arbitrary byte sequences.

Test plan

Existing unit tests

@cla-bot cla-bot bot added the cla-signed label Apr 17, 2024
@github-actions github-actions bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels Apr 17, 2024
@ggilmore ggilmore enabled auto-merge (squash) April 17, 2024 15:10
@ggilmore ggilmore merged commit 6a0c465 into main Apr 17, 2024
14 checks passed
@ggilmore ggilmore deleted the spr/main/582a4b96 branch April 17, 2024 15:33
@sourcegraph-release-bot
Copy link
Collaborator

The backport to 5.3.9104 failed at https://github.com/sourcegraph/sourcegraph/actions/runs/8724701853:

The process '/usr/bin/git' failed with exit code 1

To backport this PR manually, you can either:

Via the sg tool

Use the sg backport command to backport your commit to the release branch.

sg backport -r 5.3.9104 -p 61970
Via your terminal

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.3.9104 5.3.9104
# Navigate to the new working tree
cd .worktrees/backport-5.3.9104
# Create a new branch
git switch --create backport-61970-to-5.3.9104
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 6a0c465fbd73943cd08e37ced457e232a40c521b
# Push it to GitHub
git push --set-upstream origin backport-61970-to-5.3.9104
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.3.9104

If you encouter conflict, first resolve the conflict and stage all files, then run the commands below:

git cherry-pick --continue
# Push it to GitHub
git push --set-upstream origin backport-61970-to-5.3.9104
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.3.9104
  • Follow above instructions to backport the commit.
  • Create a pull request where the base branch is 5.3.9104 and the compare/head branch is backport-61970-to-5.3.9104., click here to create the pull request.

Once the pull request has been created, please ensure the following:

  • Make sure to tag @sourcegraph/release in the pull request description.

  • kindly remove the release-blocker from this pull request.

@sourcegraph-release-bot sourcegraph-release-bot added backports failed-backport-to-5.3.9104 release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases labels Apr 17, 2024
ggilmore added a commit that referenced this pull request Apr 17, 2024
…nstead of `string` (#61970)

commit-id:582a4b96
(cherry picked from commit 6a0c465)
@ggilmore ggilmore added failed-backport-to-5.3.9104 and removed release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases failed-backport-to-5.3.9104 labels Apr 17, 2024
jdpleiness pushed a commit that referenced this pull request Apr 17, 2024
…e `bytes` instead of `string` (#61970) (#61982)

gitserver: protobuf: relax ArchiveRequest path fields to be `bytes` instead of `string` (#61970)

commit-id:582a4b96
(cherry picked from commit 6a0c465)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants