Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
89442: build: delete `vendor` submodule r=knz,dt a=rickystewart

The `vendor` submodule is not really necessary for anything any more.
For the Bazel build, we have mirrored all of our dependencies, so
vendoring provides no additional value. `go` tooling is also generally
happy to point to the module cache for e.g. go-to-definition. Dealing
with the submodule is a pain, so it behooves us to get rid of it.

For `make` builds, the `vendor` directory will be synthesized
automatically. It is now a `gitignore`'d directory. You can also
still `make vendor_rebuild` if you want to force synthesizing the
directory. Tooling can be updated to just not use `-mod=vendor` and the
`go` module cache should be used in its place transparently.

Epic: None
Release note: None

92694: server, ui:  add multitenant login/logout and tenant dropdown r=Santamaura a=Santamaura

ui, server: add multitenant login/logout and tenant dropdown

This patch enables login/logout for all tenants on the cluster
by fanning out the incoming requests to each tenant server.
Multitenant login introduces a new multitenant session cookie
with the format as <session>,<tenant_name,<session2>,<tenant_name2>
etc. The admin ui displays a dropdown with a list of tenants
the user has successfully logged in to. Selecting a different
tenant sets the tenant cookie to the selected tenant name
and reloads the page. If the cluster is not multitenant, the
dropdown will not display.

Release note (ui change): added a top-level dropdown
on the admin ui which lists tenants the user has logged
in to. If not multitenant, the dropdown is not displayed.

Epic: https://cockroachlabs.atlassian.net/browse/CRDB-14546

93487: upgrades: fix upgrade to add statement_diagnostics_requests.completed… r=ajwerner a=ajwerner

…_idx

The migration used a different column ordering than the descriptor in the bootstrap schema. The value in the bootstrap schema is the value used to determine whether the migration succeeded successfully. In general, you can hit this bug if you upgrade from 22.1->22.2 and then you create the index with the migration but crash before the index is fully created. In that case, the code will think that it's the wrong index. This should be rare, but would be problematic. Now we've made them match.

This change also augments the roachtest which checks that the system schema looks correct to check on what happens when you upgrade from a previous snapshot. That matters here because the migration in question still exists on master, and is not idempotent. We should have found that, but didn't because we need multiple steps in the upgrade. We can get that pretty cheaply.

Fixes #93133

Release note (bug fix): Fixed a rare bug which could cause upgrades from 22.1 to 22.2 to fail if the job coordinator node crashes in the middle of a specific upgrade migration.

Co-authored-by: Ricky Stewart <rickybstewart@gmail.com>
Co-authored-by: Santamaura <alexsantamaura@gmail.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
  • Loading branch information
4 people committed Dec 13, 2022
4 parents 16c786f + 9aa666b + 141b64b + 2a118aa commit 3ef084c
Show file tree
Hide file tree
Showing 30 changed files with 781 additions and 295 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ artifacts
# fuzz tests
work-Fuzz*
*-fuzz.zip
/vendor
# vendoring by `go mod vendor` may produce this file temporarily
/.vendor.tmp.*
# The Observability Service binary.
Expand Down
3 changes: 0 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
[submodule "vendor"]
path = vendor
url = https://github.com/cockroachdb/vendored
[submodule "c-deps/jemalloc"]
path = c-deps/jemalloc
url = https://github.com/cockroachdb/jemalloc.git
Expand Down
92 changes: 49 additions & 43 deletions Makefile

Large diffs are not rendered by default.

128 changes: 7 additions & 121 deletions build/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ back to this document and perform these steps:
* [ ] Run `./dev generate bazel` to refresh `distdir_files.bzl`, then `bazel fetch @distdir//:archives` to ensure you've updated all hashes to the correct value.
* [ ] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [ ] Bump the go version in `go.mod`.
* [ ] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)).
* [ ] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
Expand Down Expand Up @@ -151,10 +151,7 @@ The `bazelbuilder` image is used exclusively for performing builds using Bazel.

# Dependencies

Dependencies are managed using `go mod`. We use `go mod vendor` so that we can import and use
non-Go files (e.g. protobuf files) using the [modvendor](https://github.com/goware/modvendor)
script. Adding or updating a dependecy is a two step process: 1) import the dependency in a go
file on your local branch, 2) push a commit containing this import to the `vendored` git submodule.
Dependencies are managed using `go mod`.

## Working Locally with Dependencies

Expand Down Expand Up @@ -184,21 +181,15 @@ file on your local branch, 2) push a commit containing this import to the `vendo

4. Import the dependency to a go file in `cockroachdb/cockroach`. You may use an anonymous
import, e.g. `import _ "golang.org/api/compute/v1"`, if you haven't written any code that
references the dependency. This ensures cockroach's make file will properly add the package(s) to the vendor directory. Note that IDEs may bicker that
these import's paths don't exist. That's ok!
references the dependency. This ensures `go mod tidy` will not delete your dependency.
Note that IDEs may bicker that these import's paths don't exist. That's ok!
5. Run `go mod tidy` to ensure stale dependencies are removed.
6. Run `make vendor_rebuild` to add the package to the vendor directory. Note this command will only
add packages you have imported in the codebase (and any of the package's dependencies), so you
may want to add import statements for each package you plan to use (i.e. repeat step 3 a couple times).
7. Run `cd vendor && git diff && cd ..` to ensure the vendor directory contains the package(s)
you imported
8. Run `make buildshort` to ensure your code compiles.
9. Run `./dev generate bazel --mirror` to regenerate DEPS.bzl with the updated Go dependency information.
6. Run `./dev generate bazel --mirror` to regenerate DEPS.bzl with the updated Go dependency information.
Note that you need engineer permissions to mirror dependencies; if you want to get the Bazel build
working locally without mirroring, `./dev generate bazel` will work, but you won't be able to check
your changes in. (Assuming that you do have engineer permissions, you can run
`gcloud auth application-default login` to authenticate if you get a credentials error.)
10. Follow instructions for [pushing the dependency to the `vendored` submodule](#pushing-the-dependency-to-the-vendored-submodule)
7. Run `./dev build short` to ensure your code compiles.

### Updating a Dependency

Expand All @@ -224,9 +215,6 @@ Note:
proceed as per "Upgrading important infrastructure dependencies"
below.

When [pushing the dependency to the `vendored` submodule](#pushing-the-dependency-to-the-vendored-submodule), you may either checkout a new branch, or create a new commit in the original branch you used to publicize the vendor
dependency.

### Updating important infrastructure dependencies

Sometimes a key, infrastructure-like dependency is upgraded as a
Expand All @@ -253,106 +241,4 @@ To achieve this, proceed as follows:

### Removing a dependency

When a dependency has been removed, run `go mod tidy` and then `make vendor_rebuild`.
Then follow the [Pushing the Dependency to the `vendored` submodule](#pushing-the-dependency-to-the-vendored-submodule) steps.

## Working With Submodules

To keep the bloat of all the changes in all our dependencies out of our main
repository, we embed `vendor` as a git submodule, storing its content and
history in [`vendored`](https://github.com/cockroachdb/vendored) instead.

This split across two repositories however means that changes involving
changed dependencies require a two step process. After altering dependencies and making related code
changes, follow the steps below.

### Pushing the Dependency to the `vendored` submodule

- Notice that `git status` in `cockroachdb/cockroach` checkout will
report that the `vendor` submodule has `modified/untracked content`.

- `cd` into `vendor`, and ...
+ Checkout a **new** named branch
+ Run `git add .`
+ Commit all changes, with a nice short message. There's no explicit policy related to commit
messages in the vendored submodule.

- At this point the `git status` in your `cockroachdb/cockroach`
checkout will report `new commits` for `vendor` instead of `modified
content`.
- Back in your `cockroachdb/cockroach` branch, commit your code
changes and the new `vendor` submodule ref.

- Before the `cockroachdb/cockroach` commit can be submitted in a pull request, the submodule commit
it references must be available on `github.com/cockroachdb/vendored`. So, when you're ready to
publicize your vendor changes, push the `vendored` commit to remote:

+ Organization members can push their named branches there directly, via:

`git push [remote vendor name, probably 'origin'] [your vendor branch] `

+ Non-members should fork the `vendored` repo and submit a pull
request to `cockroachdb/vendored`, and need wait for it to merge
before they will be able to use it in a `cockroachdb/cockroach`
PR.

### `master` Branch Pointer in Vendored Repo

Since the `cockroachdb/cockroach` submodule references individual commit
hashes in `vendored`, there is little significance to the `master` branch in
`vendored` -- as outlined above, new commits are always authored with the
previously referenced commit as their parent, regardless of what `master`
happens to be.

It is critical that any ref in vendored that is referenced from `cockroachdb/cockroach` remain
available in vendored in perpetuity. One way to ensure this is to leave the vendored branch that
you pushed your changes to in place.

If you would like to delete your feature branch in the vendored repository, you must first ensure
that another branch in vendored contains the commit referenced by `cockroachdb/cockroach`. You can
update the master branch in vendored to point at the git SHA currently referenced in
`cockroachdb/cockroach`.

### Conflicting Submodule Changes

If you pull/rebase from `cockroach/cockroachdb` and encounter a conflict in the vendor directory,
it is often easiest to take the master branch's vendored directory and then recreate your vendor
changes on top of it. For example:

1. Remove your local changes to `vendored` by resetting your local
vendor directory to the commit currently used by `origin/master` on
`cockroachdb/cockroach`.
+ Get reference: `git ls-tree origin/master vendor | awk '{print $3}'`
+ Reset to it: `cd vendor && git reset --hard REF`
2. In `cockroach/cockroachdb`, amend the commit that contained the dirty vendor pointer.
3. Try pulling/rebasing again, and if that works, rebuild your local vendor repo with
`go mod tidy` and `make vendor_rebuild`
4. Push the clean vendor changes to the remote vendor submodule, following the [Pushing the Dependency to the `vendored` submodule](#pushing-the-dependency-to-the-vendored-submodule)

Note: you may also observe conflicts in `go.mod` and `go.sum`. Resolve the conflict like
any vanilla conflict on `cockroach/cockroachdb`, preferring master's
version. Then, `make vendor_rebuild` to re-add your local changes to `go.
mod` and `go.sum`.
### Recovering from a broken vendor directory

If you happen to run into a broken `vendor` directory which is irrecoverable,
you can run the following commands which will restore the directory in
working order:

```
rm -rf vendor
git checkout HEAD vendor # you can replace HEAD with any branch/sha
git submodule update --init --recursive
```

### Repository Name

We only want the vendor directory used by builds when it is explicitly checked
out *and managed* as a submodule at `./vendor`.

If a go build fails to find a dependency in `./vendor`, it will continue
searching anything named "vendor" in parent directories. Thus the vendor
repository is _not_ named "vendor", to minimize the risk of it ending up
somewhere in `GOPATH` with the name `vendor` (e.g. if it is manually cloned),
where it could end up being unintentionally used by builds and causing
confusion.
When a dependency has been removed, run `go mod tidy` and `dev generate bazel`.
8 changes: 2 additions & 6 deletions build/teamcity-check-genfiles.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,9 @@ tc_end_block "Ensure generated code is up-to-date"

# generated code can generate new dependencies; check dependencies after generated code.
tc_start_block "Ensure dependencies are up-to-date"
# Run go mod tidy and `make -k vendor_rebuild` and ensure nothing changes.
# Run `go mod tidy` and ensure nothing changes.
run build/builder.sh go mod tidy
check_workspace_clean 'go_mod_tidy' "Run \`go mod tidy\` and \`make -k vendor_rebuild\` to automatically regenerate these."
run build/builder.sh make -k vendor_rebuild
cd vendor
check_workspace_clean 'vendor_rebuild' "Run \`make -k vendor_rebuild\` to automatically regenerate these."
cd ..
check_workspace_clean 'go_mod_tidy' "Run \`go mod tidy\` to automatically regenerate these."

end_check_generated_code_tests
tc_end_block "Ensure dependencies are up-to-date"
2 changes: 1 addition & 1 deletion build/teamcity-compose.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ tc_end_block "Compile compose tests"
tc_start_block "Run compose tests"
# NB: apply the same trick as teamcity-acceptance.sh
run_json_test stdbuf -eL -oL go test \
-mod=vendor -json -v -timeout 30m -tags compose \
-json -v -timeout 30m -tags compose \
-exec "../../build/teamcity-go-test-precompiled.sh ./pkg/compose/compose.test -artifacts \"$TMPDIR\"" ./pkg/compose
tc_end_block "Run compose tests"
1 change: 0 additions & 1 deletion build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ define VALID_VARS
GOFLAGS
GOGOPROTO_PROTO
GOGO_PROTOBUF_PATH
GOMODVENDORFLAGS
GOPATH
GORACE
GOTESTFLAGS
Expand Down
6 changes: 4 additions & 2 deletions build/vendor_rebuild.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ function restore() {
fi
}

mv vendor $TMP_VENDOR_DIR
if [ -d vendor ]; then
mv vendor $TMP_VENDOR_DIR
fi
go mod download
go mod vendor
modvendor -copy="**/*.c **/*.h **/*.proto" -include 'github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api,github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/rpc,github.com/prometheus/client_model'

mv $TMP_VENDOR_DIR/.git vendor/.git
rm -rf $TMP_VENDOR_DIR
2 changes: 1 addition & 1 deletion pkg/cli/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestNoLinkForbidden(t *testing.T) {
// The errors library uses go/build to determine
// the list of source directories (used to strip the source prefix
// in stack trace reports).
"github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/errors/withstack",
"github.com/cockroachdb/errors/withstack",
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) {
}

// expected and actual output of `SHOW CREATE ALL TABLES;`.
var expected string
var actual string
var expected, actual string

// Query node `SHOW CREATE ALL TABLES` and store return in output.
obtainSystemSchemaStep := func(node int, output *string) versionStep {
Expand All @@ -83,13 +82,13 @@ func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) {
}

// Compare whether two strings are equal -- used to compare expected and actual.
validateEquivalenceStep := func(str1, str2 string) versionStep {
validateEquivalenceStep := func(str1, str2 *string) versionStep {
return func(ctx context.Context, t test.Test, u *versionUpgradeTest) {
if str1 != str2 {
if *str1 != *str2 {
t.Fatal("After upgrading, `USE system; SHOW CREATE ALL TABLES;` " +
"does not match expected output after version upgrade.\n")
}
t.L().Printf("validating succeeded")
t.L().Printf("validating succeeded:\n%v", *str1)
}
}

Expand All @@ -116,7 +115,7 @@ func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) {
obtainSystemSchemaStep(1, &actual),

// Compare the results.
validateEquivalenceStep(expected, actual),
validateEquivalenceStep(&expected, &actual),
)
u.run(ctx, t)
},
Expand Down
1 change: 1 addition & 0 deletions pkg/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ go_library(
"server_http.go",
"server_sql.go",
"server_systemlog_gc.go",
"session_writer.go",
"settings_cache.go",
"sql_stats.go",
"start_listen.go",
Expand Down
31 changes: 8 additions & 23 deletions pkg/server/api_v2_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,41 +308,26 @@ func (a *authenticationV2Mux) getSession(
return "", nil, http.StatusUnauthorized, err
}

possibleSessions := []string{}
cookie := &serverpb.SessionCookie{}
var err error
if rawSession == apiV2UseCookieBasedAuth {
cookies := req.Cookies()
for _, c := range cookies {
if c.Name != SessionCookieName {
continue
}
possibleSessions = append(possibleSessions, c.Value)
}
cookie, err = findAndDecodeSessionCookie(req.Context(), req.Cookies())
} else {
possibleSessions = append(possibleSessions, rawSession)
}

sessionCookie := &serverpb.SessionCookie{}
var decoded []byte
var err error
for i := range possibleSessions {
decoded, err = base64.StdEncoding.DecodeString(possibleSessions[i])
decoded, err := base64.StdEncoding.DecodeString(rawSession)
if err != nil {
log.Warningf(ctx, "attempted to decode session but failed: %v", err)
continue
return "", nil, http.StatusBadRequest, err
}
err = protoutil.Unmarshal(decoded, sessionCookie)
err = protoutil.Unmarshal(decoded, cookie)
if err != nil {
log.Warningf(ctx, "attempted to unmarshal session but failed: %v", err)
continue
}
// We've successfully decoded a session from cookie or header.
break
}
if err != nil {
err := errors.New("invalid session header")
return "", nil, http.StatusBadRequest, err
}
valid, username, err := a.s.authServer.verifySession(req.Context(), sessionCookie)
valid, username, err := a.s.authServer.verifySession(req.Context(), cookie)
if err != nil {
apiV2InternalError(req.Context(), err, w)
return "", nil, http.StatusInternalServerError, err
Expand All @@ -352,7 +337,7 @@ func (a *authenticationV2Mux) getSession(
return "", nil, http.StatusUnauthorized, err
}

return username, sessionCookie, http.StatusOK, nil
return username, cookie, http.StatusOK, nil
}

func (a *authenticationV2Mux) ServeHTTP(w http.ResponseWriter, req *http.Request) {
Expand Down
Loading

0 comments on commit 3ef084c

Please sign in to comment.