* Add refresh token header/cookie
As part of work on automatic refreshing of access tokens in the grafana plugin
(FB-1377), we will now accept a refresh token in the "X-Molecula-Refresh-Token"
header or the "refresh-molecula-chip" cookie.
This refresh token will be used if the access token is expired. To achieve this,
there was a lot of plumbing that had to be done. Here is a list of some of it:
* Added lots of constants for the new values.
* Removed token cache, since we will be keeping state on the clients.
* We now only refresh tokens when they are expired, which is more inline with the
OAuth spec.
* Refactored SetGRPCMetadata to be simpler to read.
* Refactored AddAuthToken.
* Update failing tests.
* We now don't split GRPC cookies on ";". Not sure why we did that before tbh.
I also added TODOs to add the refresh token to other subcommands. This is out of
scope for my current ticket, but it would be nice to have in the future.
* remove unnecessary context from Authenticate
* Add comments on why we check both cases for headers
It's because some GRPC clients lowercase metadata names. I've run into issues with
this enough that I think it's worth the extra checks. We prefer lowercase though,
because that's "standard".
* Fix test that broke during rebase
* Use IP whitelisting for ingest
For ingest, use configured IPs to authenticate the requests.
Auth-token will no longer be used for requests from ingest consumers.
If IP in request is in configured IPs, authenticate and authorize as an admin.
If IP in request is not in configured IPs, proceed with the standard authentication/authorization using ADD.
* need to remove port from client IP
* addressed review comments
When deletion is started, _exists field is updated with row+1.
After deletion is completed, we delete _exists=row+1.
If _exists>=1, then deletion was not completed.
Updated go version in docker to match other requirements.
Removed duplicate error check for grpc.
-This code should cover the retry logic.
-The ingest is set to 100,000 records, because it's difficult to cause
leader change with lower number of records to ingest.
-Additional node: By applying stress on other two nodes while ingesting
simultaniously causes "context erorr" logic which can fail many tests.
- For server side, used an instrumented binary with a test that wraps around the main entrypoint for featurebase
- Every time, the binary is called, a new coverage file is generated.
- For the client side, used the standard -coverprofile flag for go test to generate code coverage
- For backup test that's expected to fail, needed to call Run call in backup.go directly. The code coverage is not written to disk for an instrumented binary if there is an error.
- rip out gobby stuff
- add tokenCache, groupsCache
- refresh the token if needed
- set cookies after authenticate
- remove signature validation, the IDP does that for us
- added way more unit tests
- update older tests to use new API
- add fake idp to authcluster tests
this is necessary as in some cases we want a low timeout (when we
expect a quick response, e.g. with backup), but in others we may want
a very long timeout (long running query).
Now we have more granular control over timeouts so we can get things
to fail more predictably in tests.
this should be a lot more reliable than trying to construct it based
on the project name as the exact construction can differ between
docker-compose versions.
There was also an issue with the backups succeeding when they should
fail in the test. There's an arcane maze of HTTP timeouts to navigate
here, but basically there are situations where the client will just
wait forever rather than erroring if the server is paused at the
right(wrong) time. I'm not convinced we've solved every possible case
of this, so we still may see the backup succeed even when it's
supposed to fail. The ultimate hammer is to add Client.Timeout, but
that's a very blunt instrument and I'm afraid it could cause a timeout
when really we just have a lot of data to download or something.
There may be a better way to say "only time out if you literally
haven't heard a peep from the server in this long", but I haven't been
able to figure it out yet.
I also fixed how the authclustertests are run as they weren't using
the PROJECT parameter correctly. Now they can run concurrently with
clustertests, and with other copies of authclustertests without having
conflicts.
- Add auth-token for featurebase import, backup and restore
- Add auth-token to http request
- Create a cluster tests with auth enabled
- Add test for import with auth enabled
A few things were going wrong here.
First, we take a "RetryPeriod" option on backup and restore which is
meant to be roughly the total amount of time we spend retrying any
given request before failing. However we were incorrectly passing that
as the RetryMaxWait which is the maximum amount of time to sleep
between any two attempts. We now do some fuzzy math to figure out
approximately how many attempts we should make given a minimum sleep
of 100ms and the fact that we double the sleep time every attempt.
Second, during the backup test, if a host was totally stopped when we
started the request, it would fail immediately and then retry, but if
the host was stopped during the request (after DNS had resolved), then
the request would wait for the DialTimeout which we default to 30s, so
turning off the cluster for 5 seconds and turning it back on resulted
in the backup completing rather than failing. Because of this, we
change the commandClient to have a default dial timeout of 1 second.
I was tempted to change the global default to 1s which I think would
be fine, but didn't want to break anything too badly.
i used this script, a little clunky but it got the job done
```bash
for file in `find . -type f -print | grep '\.go'`; do
sed '1,/^\/\/ limitations under the License.$/d' $file > $file.tmp;
result=`cat $file.tmp`
if [[ result != "" ]]; then
gofmt $file.tmp &> /dev/null;
if [[ $? == 0 ]]; then
mv $file.tmp $file && gofmt -w $file;
else
rm $file.tmp;
fi
else
rm $file.tmp;
fi
done
```
Underlying goal: Don't use the http client to send messages back to the
local host. Also, when sending data to other nodes, don't collate it
from an ImportRequest into a completely different format, then immediately
collate that back into an ImportRequest. This does require changing
the logic over in ctl/import to make it create an ImportRequest.
Also, add additional testing to make sure we're actually trying anything
at all with several combinations (such as submitting import requests
which don't match the configuration of index or field), and improve
test coverage for that.
This introduces the ability to tell an http/client InternalClient about
a specific API that it should use for local queries where applicable.
That's not implemented outside of the import stuff, but should probably
be applied eventually to other things that are trying to talk to many
nodes one of which may be the local node. That behavior is contingent
on passing in a Qcx, because it is implicitly tied to an existing
execution context, and it can't assume that it can create a new one,
because that could deadlock.
Performance of tests on MacOS has been atrocious for a while, and
a lot of that is fsync, so we're trying to make that optional.
To test all of this, I modified RBF to panic if anything tried to
open an RBF database without disabling fsync, and ran the tests that
way, and tracked down the various places this could still happen.
There's a lot of places in our tree where we were creating
test holders which were not getting created with fsync disabled, which
results in a surprisingly large number of points at which we end
up calling fsync in tests, which makes tests much slower than they
need to be. There's also a bunch of places where the flags don't get
propagated correctly; for instance, storage.fsync didn't propagate
to the RBFConfig.
We add an "fsync enabled" flag to OpenTranslateStoreFunc, so we can
tell translation stores that we don't need syncing, so the server's
config can be passed on appropriately.
More of the test code that sets things up is correctly configuring
that flag by default.
We also change the barely-used bolt storage backend to support this as
well.
With this done, the only calls to fsync left in a run of `go test -short`
in the top-level directory are from the zap logger in etcd, and consumed
around 0.03 seconds. The overall impact is that `go test -short`
went from "takes enough more than 10 minutes that i don't know how long
it takes" to about 2.5 minutes.
This adds the following test:
1. cluster comes up (node 1,2,3), status normal
2. Pause node 3
3. Insert keys making sure to filter out the keys that will go to the paused node
4. Wait for status to become degraded
5. Unpause node 3
6. Wait for status to get back to normal
7. Check that keys were replicated to all 3 nodes
