Skip to content

Support Workhorse config options for propagating correlation IDs

Stan Hu requested to merge sh-workhorse-correlation-id-propagate-config into master

What does this MR do?

This commit adds support for two Workhorse configuration options added in gitlab-org/gitlab!66715 (merged):

  1. workhorse.trustedCIDRsForPropagation
  2. workhorse.trustedCIDRsForXForwardedFor

Note that workhorse.extraArgs must also include -propagateCorrelationID.

These configuration options make it possible to trace the entire flow of the request via correlation_id. Previously when Gitaly made a request to the internal API, Workhorse would generate a new correlation ID. As a result, we would lose the ability to trace every RPC made during a UI edit, for example.

workhorse.trustedCIDRsForXForwardedFor tells Workhorse what remote IPs can be trusted to use the X-Forwarded-For HTTP header to resolve the actual client IP. Note that this parameter is only used to determine whether to propagate the correlation ID. It is not yet used for logging the remote IP resolution, but it should be.

workhorse.trustedCIDRsForXForwardedFor allows Workhorse to restrict propagation to certain IP ranges. We will want to add Gitaly servers and other services that make HTTP internal calls (e.g. GitLab Pages) to this list.

Related issues

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion.

Required

  • Merge Request Title and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Tests added
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for omnibus-gitlab opened

Tests

This was my values:

diff --git a/charts/gitlab/charts/webservice/values.yaml b/charts/gitlab/charts/webservice/values.yaml
index 438b9ed69..18fbfe606 100644
--- a/charts/gitlab/charts/webservice/values.yaml
+++ b/charts/gitlab/charts/webservice/values.yaml
@@ -3,7 +3,7 @@
 # Declare variables to be passed into your templates.
 image:
   pullSecrets: []
-  # pullPolicy: IfNotPresent
+  pullPolicy: Always
   # repository: registry.gitlab.com/gitlab-org/build/cng/gitlab-webservice-ee
   # tag: master
 init:
@@ -120,10 +120,10 @@ deployment:
 
 workhorse:
   keywatcher: true
-  # trustedCIDRsForPropagation: ["127.0.0.1/32"]
-  # trustedCIDRsForXForwardedFor: ["127.0.0.1/32"]
+  trustedCIDRsForPropagation: ["192.168.0.0/16", "127.0.0.1/32"]
+  trustedCIDRsForXForwardedFor: ["192.168.0.0/16", "127.0.0.1/32"]
   sentryDSN: ""
-  extraArgs: ""
+  extraArgs: "-propagateCorrelationID"
   logFormat: json # valid: (json, structured, text)
   resources:
     requests:

Then, I modified a file in the repository. The Gitaly logs show that the correlation_id (ca462b846900dd98022722cf5e3f067e in this case) stayed the same through the whole request, allowing us to trace the entire flow of the request:

$ kubectl logs gitlab-gitaly-0 | grep ca462b846900dd98022722cf5e3f067e |  jq | egrep "grpc.method|url"
  "grpc.method": "FindCommit",
  "grpc.method": "TreeEntry",
  "grpc.method": "FindCommits",
  "grpc.method": "FindCommits",
  "grpc.method": "RepositoryExists",
  "grpc.method": "Cleanup",
  "grpc.method": "Cleanup",
  "grpc.method": "CommitIsAncestor",
  "grpc.method": "ListLFSPointers",
  "url": "http://gitlab-webservice-default.default.svc:8181//api/v4/internal/allowed"
  "url": "http://gitlab-webservice-default.default.svc:8181//api/v4/internal/pre_receive"
  "url": "http://gitlab-webservice-default.default.svc:8181//api/v4/internal/post_receive"
  "grpc.method": "UserCommitFiles",
  "grpc.method": "UserCommitFiles",
  "grpc.method": "FindLocalBranches",
  "grpc.method": "RepositorySize",
  "grpc.method": "FindAllBranchNames",
  "grpc.method": "HasLocalBranches",
  "grpc.method": "CountCommits",
  "grpc.method": "ApplyGitattributes",
  "grpc.method": "ApplyGitattributes",
  "grpc.method": "CommitsBetween",
  "grpc.method": "CommitDelta",
  "grpc.method": "FindCommit",
  "grpc.method": "FindAllTagNames",
  "grpc.method": "TreeEntry",
  "grpc.method": "TreeEntry",
  "grpc.method": "ListCommitsByOid",
  "grpc.method": "RefExists",
  "grpc.method": "ReferenceTransactionHook",
  "grpc.method": "ReferenceTransactionHook",
  "grpc.method": "WriteRef",
  "grpc.method": "WriteRef",
  "grpc.method": "FindCommit",
  "grpc.method": "RefExists",
  "grpc.method": "FindCommit",
  "grpc.method": "FilterShasWithSignatures",
  "grpc.method": "FindCommit",
  "grpc.method": "GetTreeEntries",
  "grpc.method": "CommitLanguages",
  "grpc.method": "CommitLanguages",
$ kubectl logs gitlab-webservice-default-5d587b8bd6-875qw -c gitlab-workhorse | grep ca462b846900dd98022722cf5e3f067e | jq | grep uri
  "uri": "//api/v4/internal/pre_receive",
  "uri": "//api/v4/internal/post_receive",
  "uri": "/root/test/-/update/main/.gitlab-ci.yml",
$ kubectl logs gitlab-webservice-default-5d587b8bd6-rs7x2 -c gitlab-workhorse | grep ca462b846900dd98022722cf5e3f067e | jq | grep uri
  "uri": "//api/v4/internal/allowed",
Edited by Stan Hu

Merge request reports