This change is fixing "All Java build rules should be loaded from
Starlark" warning flagged by latest buildifier version: [1]. Python
rules are now also loaded from the Starlark.
Also extract codemirror library import to BUILD file. This is needed to
avoid cycle in the workspace file, after importing java rules from
Starlark.
[1] https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#native-java
Change-Id: I36192c9465d988b25cf09c250e110f15850910cd
This new version has the nice feature of putting multiple chained method
calls before a .stream() on a single line. The diff is a bit large but
it's removing a lot of newlines which is nice.
Change-Id: I260b620aa6a1bc77b06be9672a1f281ab0d0d0f8
Pass the right access path for the commands executed in Git
because of an incoming SSH connection:
- GIT for all Git/SSH commands
- SSH_COMMAND for everything else
Allow to honour force push ACL (and potentially other operations)
because of the correct propagation of the original identity
with the right access path.
Previously the code was setting the access path on SshScope.Context
which is stored in the ThreadLocal context of the receiving SSH command,
which is different from the other thread that execute the action.
Bug: Issue 9823
Change-Id: I2b9e1369e53a000457d4571ecf5e6d7ddf843827
This reverts commit f6e7913d143b4e35c6d17ec9b592d9820a7132d8.
Reason for revert: propagating the Context across threads
created DB leaks.
Bug: Issue 9836
Change-Id: I439ea0c4c6ea24346f0bbd3a721bdd760844b110
Honour force push ACL (and potentially other operations) by propagating
SshScope context which includes the original identity of the calling user
with its associated access path.
Previously the code was relying on ThreadLocal and thus was able to propagate
the context from parent to child threads. Now it needs to be done explicitly.
Bug: Issue 9823
Change-Id: I9eef144eee0d5831d38c4174e22018458db67669
Reformat all files with the exception of:
- lib/asciidoctor/java/AsciiDoctor.java
to avoid requiring the Library-Compliance label.
Change-Id: Iedba3f24ac00e2186e3e0688fabea817ddf43739
The following queues will provide metrics:
* index_batch
* index_interactive
* receive_commits
* send_email
* ssh_batch_worker
* ssh_command_start
* ssh_interactive_worker
* ssh_stream_worker
For each queue following metrics are available:
* active_threads - Number of threads that are actively executing tasks
* max_pool_size - Maximum allowed number of threads in the pool
* pool_size - Current number of threads in the pool
* scheduled_tasks - Number of scheduled tasks in the queue
* total_completed_tasks_count - Total number of tasks that have completed execution
* total_scheduled_tasks_count - Total number of tasks that have been scheduled
Change-Id: I9430c2a90d1fb07630c1c8b599abf6dd18e01b2d
Signed-off-by: Eryk Szymanski <eryksz@gmail.com>
* 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
