* changes:
Introduce mechanism to overload Gerrit core modules
Move default memory cache implementation out of H2 package
Introduce CacheImpl annotation
Untangle persistent/memory cache implementations from each other
H2CacheFactory: update internal caches list synchronously
All logging of this class should be done through the locally defined
'sshDaemonLog' Logger (and not through the inherited 'log' Logger from
AbstractLoggingBean).
Change-Id: I2d1031fe51601f11911b110c482482ff2482ce1a
Signed-off-by: Edwin Kempin <ekempin@google.com>
(cherry picked from commit 43e3c48bbce0a8b2760f5e9f8e9615a1164a4fc1)
The memory cache doesn't depend on H2 at all, and this separation of
packages makes it harder to reintroduce the kind of source-level
dependency that was removed in I570fd54adf.
Change-Id: Ie23e39cd7ea9a0813a914e65614b889c248342ae
Binding the class with in(SINGLETON) creates a singleton per injector,
and because it is done in two injectors (the SSH injector and the HTTP
injector), we end up with two instances and thus two work queues for
both SSH-Interactive-Worker and SSH-Batch-Worker.
Remove the in(SINGLETON) and instead explicitly make the class singleton
by annotating with @Singleton
Change-Id: Ifa782b7ea7f97d88d0afdfa9efc87ad7baa15b93
Signed-off-by: Eryk Szymanski <eryksz@gmail.com>
Signed-off-by: David Pursehouse <dpursehouse@collab.net>
These commands are only interacting with the AbstractVersionManager, if
it is not available then they have no reason to be available.
Because those commands were always binded, binding an implementation of
AbstractVersionManager was mandatory otherwise Guice would have
complained. For that reason, the implementation of
AbstractVersionManager was done even in the case when index is a single
version and an implementation of AbstractVersionManager is not required.
Only bind implementations of AbstractVersionManager when needed and bind
the commands only if AbstractVersionManager implementation is binded.
Change-Id: I3db48f6b5d030ecc6efa840c59eb4e177ae1b6b1
Before this fix, using the IndexStartCommand [1] while passing an
invalid or unknown index name with the --force option led to an NPE in
the log, thus a fatal error in user's console. Also, using an unknown
index name either without --force or through the sibling
IndexActivateCommand [2] used to display an inaccurate console message.
This fix always shows a clearer message to the user, upon passing an
unknown index name to either [1] or [2]. It also now prevents that NPE.
[1] https://gerrit-review.googlesource.com/Documentation/cmd-index-start.html
[2] https://gerrit-review.googlesource.com/Documentation/cmd-index-activate.html
Bug: Issue 8715
Change-Id: I16cb800fbe9af00e47bb22105d1c4d4bf044fdc5
Before this fix, the IndexStartCommand and IndexActivateCommand worked
only for the Lucene index-type case. Replace the latter hard-coded case
with support for it but also the Elastic case as well. Meaning, use
AbstractVersionManager rather than its LuceneVersionManager sub-type, so
that ElasticVersionManager can also be (seamlessly) supported for those
commands. Hence bind AbstractVersionManager to the configured sub-type
class, in order for it to be used that way.
This change can be fully tested through using the --force option of the
gerrit index start command [1]. The gerrit index activate command [2]
does not have such an option.
[1] https://gerrit-review.googlesource.com/Documentation/cmd-index-start.html
[2] https://gerrit-review.googlesource.com/Documentation/cmd-index-activate.html
Bug: Issue 8584, Issue 8585
Change-Id: Ibd8d22dca3a94e8b6a81db2935d1c34b1e80401d
This reverts partially commit a0ae281aa9266e0a65555cc80d838fbef3839225.
That commit prevent multi line arguments for ending up in the error_log,
output of show-queue command and in sshd_log by trimming the multi line
arguments. This commit reverts only the sshd_log part.
The original motivation for the other commit was mainly for error_log
and output of show-queue, same logic was applied to sshd_log for
consistency reasons. Trimming the multi line from sshd_log was a mistake
because we lose information that can be found no where else.
Change-Id: Ib9e5bdd7a935a2a0c969deb0c0e557bef976d251
The task name is being generated as the command name followed by the
trimmed arguments, but without any space, for example:
SSH gerrit plugin rmreadonly (admin)
Fix it so that there is a space:
SSH gerrit plugin rm readonly (admin)
Change-Id: Ib5e0409a68527d810df25cd621e0e62f4442c02e
Add a new interface, SshCreateCommandInterceptor, which is bound as
a dynamic item. A plugin may implement this interface to intercept
creation of ssh commands and override them with the plugin's own.
This is useful for example to allow a plugin to block execution of
certain ssh commands.
Change-Id: Id44e225f900da918db23921803e14edd4f2350ee
Previously, the task name of review command could be multi line string
because of --message argument.
Keep only the first line of the multi-line arguments to improve
readability for the task name in output of show-queue command and logs.
This applies to any multi line argument of any command, not only to
review command.
Change-Id: Ic8f3c167a73fae9f5f883e8cc3b43f84647e1b8d
This reverts commit 9dd0e0855aa53ba46834c94102dd0de772da7f70.
The change was created to remove the unreadable multi line logs in
error_log for the review command by setting the task name(thread name)
to "gerrit review" instead of the command line. Because task name is
also used in the output of show-queue command, that change is also
affecting the output of show-queue for review tasks.
Getting rid of all the arguments is removing information that could be
useful, example which change the review command is for. This is not
really an issue for the thread name in error_log but it could become one
for the task name is output of show-queue command.
Instead of removing all the arguments of the review command, the
proposal is to revert that change and merge the following one which
keeps all the arguments but trims the multi line ones.
Change-Id: I046951a42ef6271d3f960086e1acef9f4b643d5c
Session start time should be displayed as MMM-dd if it's more than one
day ago but it's not. Check for session age is done incorrectly.
Change-Id: I86c000b42661ee01f5893d571ea77490fc1fce13
There is concern that this new feature might introduce regressions on
the stable branch. Revert it so that it can be done again on master.
This reverts commit a8a7a84ae55dd9b70cce8d7eb1dfbfd43600893a.
This reverts commit dfa74b835f451461a33c8a93c909fef11e04ff86.
Change-Id: I21e2046dfcdbba7d5f4c2704b620ea6ea689abc8
Subclasses of BaseCommand may not initialize argv field (e.g.
ScpCommand).
Recent change [1] uses argv field which may result in NPE.
This change will guard the code from such error.
[1]: https://gerrit-review.googlesource.com/c/gerrit/+/146191
Change-Id: I6dd3afa08fd696d25741b5bdec8530deac422261
Signed-off-by: Ardo Septama <aseptama@gmail.com>
If there is password in ssh command parameters, it will be logged by
Gerrit.
This patch introduce @SensitiveData annotation to mark parameters that
contain sensitive data and hide them from logger and audit.
Audit masking must be enabled at gerrit.config:
audit.maskSensitiveData=true
Change-Id: I1f0e8e91bf971ed58b2362f89ce9f02cd8fb2ec7
Signed-off-by: Ardo Septama <aseptama@gmail.com>
Task name is otherwise set to the command line which in ReviewCommands
case can be a multi line string which leads to pretty unreadable and
bordering on unparseable "loglines" like:
[2018-01-10 09:59:17,619] [SSH gerrit review 12345,4 --message 'Build:
http://jenkins.company.com/job/JOB_NAME/job/identifier/182/ : UNSTABLE
http://jenkins.company.com/job/JOB_NAME/job/other/13677/ : SUCCESS'\
--verified -1 --code-review 0 (jenkins)] WARN <class> : <message>
Change-Id: I74a0ff062b38a1980c4781e8e79a368bf028c549
Command line is not the best task description for all commands. The
"review" command command line are f.i. more often than not multiline.
This allows sub command to override task description.
Change-Id: I2bc42010fa91e94ea079f0e759c8ab049a61f462
Suexec command create a sub-context in order to execute the command as
the user to impersonate. Each context (main context and sub-context)
have their own RequestCleanup which takes care of cleaning up resources
opened in the context of the ssh request, like closing open database
connection.
The problem was the following:
-suexec command line is parsed, one db connection is opened (this
connection belongs to the main context RequestCleaner)
-sub-context is created
-command is executed in the sub-context, another database connection is
opened (this connection belongs to the sub-context RequestCleaner)
-command complete, sub-context RequestCleaner run method is called,
sub-context database connection is closed
-main context RequestCleaner run method is never called
-one database connection is leaked every time suexec command is executed
In the method to create the sub-context, there was some code trying to
prevent that problem by adding the RequestCleanup of the sub-context
into the main context RequestCleanup. The problem was that it should
have been the other way around, the main context RequestCleanup needs to
be added to the sub-context one.
The only other commands using the sub-context are Git over ssh commands
and they did not suffer from this connection leak because they do not
open database connections in the main context, only from the
sub-context.
Bug: Issue 5386
Change-Id: I7e32cedd08268b99a7bb6ffce902bf5d9fb255da
On slaves the test-submit SSH command was bound to
NotSupportedInSlaveModeFailureCommand. While this resulted in a nice
error message, confusingly this caused the test-submit command to appear
in the list of available commands when running 'gerrit --help' on
slaves. Remove the binding on slaves so that the test-submit command is
not listed as available command. This is consistent with how all other
SSH commands that are not available on slaves are handled.
Change-Id: Icb1e1ed79c0f0472d93a133516ec50b19c353d06
Signed-off-by: Edwin Kempin <ekempin@google.com>
Gerrit slaves are supposed to be read-only but the ban-commit command
creates a Git note and hence writes to the repository.
Change-Id: Ic6e43d88cfc22877256e8a6f8d8ee3673c0664e8
Signed-off-by: Edwin Kempin <ekempin@google.com>
* stable-2.13:
Fix typo in waitTimeout configuration and clarify its use
SshDaemon: introduce sshd.waitTimeout to set WAIT_FOR_SPACE_TIMEOUT
Change-Id: I9d8b69a5dbbf3a5f173285b463e6c4c92b1bdbe5
sshd introduced a new channel property
'channel-output-wait-for-space-timeout' [1] set by default to 30s.
The property isn't exposed at the ChannelSessionFactory level and thus
the default value remained hardcoded in the properties.
Without this config the consequence is that clones that requires the
server process to spend over 30s are failing.
Allow administrators to configure this via a new setting sshd.waitTimout.
Default to 30 seconds if it is not set.
[1] https://issues.apache.org/jira/browse/SSHD-565
Bug: Issue 7425
Change-Id: Ib3fd9a25d7eaaa87a15d5c159995e09a9581dadb
When executing the command:
ssh user@host gerrit stream-events --help
the onExit and destroy methods attempt to remove the event listener
registration which has not been initialized, resulting in NPE and
the command hanging.
Add null checks to prevent this.
Change-Id: I1b9ccd41d017e62f0a4206114ab15faa6ed8e000
As described in SSHD-715 [1] the NIO2_READ_TIMEOUT parameter was
introduced in sshd version 1.3.
[1] https://issues.apache.org/jira/browse/SSHD-715
Bug: Issue 6173
Change-Id: I6f930cafef9583a83aed2e6d05ff2a9f27c33cb8
FileInputStream and FileOutputStream rely on finalize() method to ensure
resources are closed. This implies they are added to the finalizer queue
which causes additional work for the JVM GC process.
This is an open bug on the OpenJDK [1] and the recommended workaround is
to use the Files.newInputStream and Files.newOutputStream static methods
instead.
[1] https://bugs.openjdk.java.net/browse/JDK-8080225
Change-Id: I3cef6fcf198dde2be7cd15bded8d2fa247177654
Since we now ship BouncyCastle in the .war file, this will always
return true. Remove the code that is now redundant.
Change-Id: I35d6191b6f5e4cea40a022236cbc848eb01d7ba1
Include the key string, which will help to track down which one
failed, and omit the entire stack trace, which doesn't actually
provide any useful information.
Change-Id: I67c1fbe75c99f8cda6dbebe27c050e338e571315
Implement online reindexing for ElasticSearch based on the code for
Lucene online reindex.
Testing scenario:
1. Start fresh Gerrit site with this patch
2. Create account
3. Verify data in ElasticSearch:
curl http://localhost:9200/gerritaccounts_0004/
curl http://localhost:9200/gerritaccounts_0004/_search
4. Stop Gerrit
5. Cherry pick change I77e1643cd1a7fbef9f4d2fa214823759188e9592
6. Start Gerrit
6. Wait for log message:
Starting online reindex from schema version 4 to 5
7. Verify state in ElasticSearch:
curl http://localhost:9200/gerritaccounts_0005/
curl http://localhost:9200/gerritaccounts_0005/_search
Entry for user account created in step 2 should have "elastic_online"
property with value "reindex work".
Change-Id: I9efcf5735e65b4f2dc2a97914d398f81656fc12a
REST API and UI allow project owner to change the project settings so
fix inconsistency by allowing the same in the ssh command.
Change-Id: I123007629db87c1df6162cb1e56fc51bacff9631
We cannot shade bouncycastle in the plugin API. Still we need it to be
included in the gerrit.war, licenses file and Eclipse classpath.
Expose bouncycastle libraries in PLUGIN_TEST_DEPS constant, so that
the plugins don't need to change anything in tree build mode.
gerrit_api() bazlet in bazlets repository is extended too, so that the
plugins don't need to change anything in standalone build mode.
One side effect of this change, is that bouncycastle libraries are
now listed with neverlink suffix, e.g.:
* bouncycastle:bcprov-neverlink
Bug: Issue 5826
Change-Id: Idb8051e16b14e20c8dd528783ab297ee25707bb3
When Gerrit SSH Daemon is stopped, there is no value in keeping its
executors threads alive as it would only consume precious resources
we do need during our Integration Tests suite.
Apache SSHD does not manage the shutdown of the internal executors
by himself, so we need to close them manually.
Change-Id: I09a62759769bbb222abd4a3ea60be8b8c5571ac9
The LfsPluginAuthCommand is only installed when lfs.plugin is set in the
gerrit.config file. However, if the corresponding plugin is not actually
installed, or it does not provide the command implementation, Failure is
thrown which results in a stack trace being emitted to the log on every
ssh push operation.
Change it to throw UnloggedFailure, and write a less verbose message to
the log at warn level. This will still be emitted to the log on every
ssh push, but it's less noisy than before.
Change-Id: Ie638bda96ddbac173eccd74f9f6a25ed87c778c8
Gerrit users who are delegated to maintain the server may be highly
restricted to access projects and branches.
There are maintenance operations that can still be allowed
on change-ids even on non-visible projects.
By allowing users delegated to maintain the server to find
non-visible change-ids in the CLI we can enable features such as
the ad-hoc change indexing.
Change-Id: Ia2ec07517c70f2cc4aed1bee832ea04a45fc6466
The Gerrit SSH command "index changes" may receive change numbers that
cannot be found or refer to repositories that have problems.
The assumption of "DB is down" when receiving an OrmException was wrong
because the JGit errors were simply wrapped as OrmException and
do not necessarily mean that you cannot continue to process the other
ones in the list.
Change-Id: I1098be16b682dce8d4e63120e7e50f937b19a7b8
(cherry picked from commit 9a52dd1dce8a79b90e8bfad4f718659aafe1a3e8)
Display a warning when a change is not found but then
continue to try to reindex the rest of the changes, consistently
with the current behavior of the command.
Change-Id: I51b98a8bb28362456ac7a5df5efa74c1fa1de284
In NoteDb external IDs are stored in the All-Users repository in a Git
Notes branch called refs/meta/external-ids where the sha1 of the
external ID is used as note name. Each note content is a Git config
file that contains an external ID. It has exactly one externalId
subsection with an accountId and optionally email and password:
[externalId "username:jdoe"]
accountId = 1003407
email = jdoe@example.com
password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
Storing the external IDs in a Git Notes branch with using the sha1 of
the external ID as note name ensures that external IDs are unique and
are only assigned to a single account. If it is tried to assign the
same external ID concurrently to different accounts, only one Git
update succeeds while the other Git updates fail with LOCK_FAILURE.
This means assigning external IDs is also safe in a multimaster setup
if a consensus algorithm for updating Git refs is implemented (which
is needed for multimaster in any case). Alternatively it was
considered to store the external IDs per account as Git config file in
the refs/users/<sharded-id> user branches in the All-Users repository
(see abandoned change 9f9f07ef). This approach was given up because in
race conditions it allowed to assign the same external ID to different
accounts by updating different branches in Git.
To support a live migration on a multi-master Gerrit installation, the
migration of external IDs from ReviewDb to NoteDb is done in 2 steps:
- part 1 (this change):
* always write to both backends (ReviewDb and NoteDb)
* always read external IDs from ReviewDb
* upgraded instances write to both backends, old instances only
write to ReviewDb
* after upgrading all instances (all still read from ReviewDb)
run a batch to copy all external IDs from the ReviewDb to NoteDb
- part 2 (next change):
* bump the database schema version
* migrate the external IDs from ReviewDb to NoteDb (for single instance
Gerrit servers)
* read external IDs from NoteDb
* delete the database table
With this change reading external IDs from NoteDb is not implemented
yet. This is because the storage format of external IDs in NoteDb
doesn't support efficient lookup of external IDs by account and this
problem is only addressed in the follow-up change (it adds a cache for
external IDs, but this cache uses the revision of the notes branch as
key, and hence can be only implemented once the external IDs are fully
migrated to NoteDb and storing external IDs in ReviewDb is dropped).
The ExternalIdsUpdate class implements updating of external IDs in
both NoteDb and ReviewDb. It provides various methods to update
external IDs (e.g. insert, upsert, delete, replace). For NoteDb each
method invocation leads to one commit in the Git notes branch.
ExternalIdsUpdate has two factories, User and Server. This allows to
record either the calling user or the Gerrit server identity as
committer for an update of the external Ids.
External IDs are now represented by a new AutoValue class called
ExternalId. This class replaces the usage of the old gwtorm entity
AccountExternalId class. For ExternalId scheme names are the same as for
AccountExternalId but no longer include the trailing ':'.
The class ExternalIdsOnInit makes it possible to update external IDs
during the init phase. This is required for inserting external IDs for
the initial admin user which is created by InitAdminUser. We need a
special class for this since not all dependencies of ExternalIdsUpdate
are available during init.
The class ExternalIdsBatchUpdate allows to do batch updates to
external IDs. For NoteDb all updates will result in a single commit to
the refs/meta/external-ids Git notes branch.
LocalUsernamesToLowerCase is now always converting the usernames in a
single thread only. This allows us to get a single commit for the
username convertion in NoteDb (this would not be possible if workers
do updates in parallel). Since LocalUsernamesToLowerCase is rather
light-weight being able to parallelize work is not really needed and
removing the workers simplifies the code significantly.
To protect the refs/meta/external-ids Git notes branch in the All-Users
repository read access for this ref is only allowed to users that have
the 'Access Database' global capability assigned. In addition
there is a commit validator that disallows updating the
refs/meta/external-ids branch by push. This is to prevent that the
external IDs in NoteDb diverge from the external IDs in ReviewDb while
the migration to NoteDb is not fully done yet.
Change-Id: Ic9bd5791e84ee8d332ccb1f709970b59ee66b308
Signed-off-by: Edwin Kempin <ekempin@google.com>
So far, indexing a series of changes had to be done by listing them one
after the other which can be cumbersome when the number of changes to
re-index is high.
Add the possibility of indexing all the changes belonging to a project.
Change-Id: Ib47f6d65ca49c85713d30806d7579f9d2d9cf823
This differs from the behavior of %r= in the magic branch or even
the new push option support of "-o r=". Drop the visibility check
here and just pass down the identities to ReceiveCommits so that
behavior is consistent.
Change-Id: Iabec4d5c48afd4e820d6bd824c6ae24b0a1e37b5
