Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher: Remove extraneous auth classes #32300

Merged
merged 6 commits into from
Aug 14, 2018
Merged

Conversation

hub-cap
Copy link
Contributor

@hub-cap hub-cap commented Jul 23, 2018

The auth.basic package was an example of a single implementation
interface that leaked into many different classes. In order to clean
this up, the HttpAuth interface, factories, and Registries all were
removed and the single implementation, BasicAuth, was substituted in all
cases. This removes some dependenies between Auth and the Templates,
which can now use static methods on BasicAuth. BasicAuth was also moved
into the http package.

The auth.basic package was an example of a single implementation
interface that leaked into many different classes. In order to clean
this up, the HttpAuth interface, factories, and Registries all were
removed and the single implementation, BasicAuth, was substituted in all
cases. This removes some dependenies between Auth and the Templates,
which can now use static methods on BasicAuth. BasicAuth was also moved
into the http package and all of the other classes were removed.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -264,12 +259,7 @@ public Watcher(final Settings settings) {
new WatcherIndexTemplateRegistry(settings, clusterService, threadPool, client);

// http client
Map<String, HttpAuthFactory> httpAuthFactories = new HashMap<>();
httpAuthFactories.put(BasicAuth.TYPE, new BasicAuthFactory(cryptoService));
// TODO: add more auth types, or remove this indirection
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@spinscale spinscale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left one comment regarding use of objectparser - which I prefer because pull parsing always means you have to take care of all the error conditions manually.

otherwise it looks fine

@@ -103,6 +98,20 @@ public static BasicAuth parse(XContentParser parser) throws IOException {
return new BasicAuth(username, password);
}

public static BasicAuth parse(XContentParser parser) throws IOException {
String type = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using an object parser here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not new, it was just yanked from HttpAuthRegistry.java since its still used in ser/deser of HttpRequest and some other classes. Id be happy to do it, but id like to make it a diff commit since this is just moving the code.

@hub-cap hub-cap merged commit e1206a6 into elastic:master Aug 14, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 14, 2018
…e-types

* elastic/master: (199 commits)
  Watcher: Remove unused hipchat render method (elastic#32211)
  Watcher: Remove extraneous auth classes (elastic#32300)
  Watcher: migrate PagerDuty v1 events API to v2 API (elastic#32285)
  [TEST] Select free port for Minio (elastic#32837)
  MINOR: Remove `IndexTemplateFilter` (elastic#32841)
  Core: Add java time version of rounding classes (elastic#32641)
  Aggregations/HL Rest client fix: missing scores (elastic#32774)
  HLRC: Add Delete License API (elastic#32586)
  INGEST: Create Index Before Pipeline Execute (elastic#32786)
  Fix NOOP bulk updates (elastic#32819)
  Remove client connections from TcpTransport (elastic#31886)
  Increase logging testRetentionPolicyChangeDuringRecovery
  AwaitsFix case-functions.sql-spec
  Mute security-cli tests in FIPS JVM (elastic#32812)
  SCRIPTING: Support BucketAggScript return null (elastic#32811)
  Unmute WildFly tests in FIPS JVM (elastic#32814)
  [TEST] Force a stop to save rollup state before continuing (elastic#32787)
  [test] disable packaging tests for suse boxes
  Mute IndicesRequestIT#testBulk
  [ML][DOCS] Refer to rules feature as custom rules (elastic#32785)
  ...
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 14, 2018
…listeners

* elastic/master:
  Watcher: Remove unused hipchat render method (elastic#32211)
  Watcher: Remove extraneous auth classes (elastic#32300)
  Watcher: migrate PagerDuty v1 events API to v2 API (elastic#32285)
  [TEST] Select free port for Minio (elastic#32837)
  MINOR: Remove `IndexTemplateFilter` (elastic#32841)
  Core: Add java time version of rounding classes (elastic#32641)
  Aggregations/HL Rest client fix: missing scores (elastic#32774)
  HLRC: Add Delete License API (elastic#32586)
  INGEST: Create Index Before Pipeline Execute (elastic#32786)
  Fix NOOP bulk updates (elastic#32819)
  Remove client connections from TcpTransport (elastic#31886)
  Increase logging testRetentionPolicyChangeDuringRecovery
  AwaitsFix case-functions.sql-spec
  Mute security-cli tests in FIPS JVM (elastic#32812)
  SCRIPTING: Support BucketAggScript return null (elastic#32811)
  Unmute WildFly tests in FIPS JVM (elastic#32814)
  [TEST] Force a stop to save rollup state before continuing (elastic#32787)
  [test] disable packaging tests for suse boxes
  Mute IndicesRequestIT#testBulk
  [ML][DOCS] Refer to rules feature as custom rules (elastic#32785)
jasontedor added a commit that referenced this pull request Aug 15, 2018
* elastic/master:
  Revert "cluster formation DSL - Gradle integration -  part 2 (#32028)" (#32876)
  cluster formation DSL - Gradle integration -  part 2 (#32028)
  Introduce global checkpoint listeners (#32696)
  Move connection profile into connection manager (#32858)
  [ML] Temporarily disabling rolling-upgrade tests
  Use generic AcknowledgedResponse instead of extended classes (#32859)
  [ML] Removing old per-partition normalization code (#32816)
  Use JDK 10 for 6.4 BWC builds (#32866)
  Removed flaky test. Looks like randomisation makes these assertions unreliable.
  [test] mute IndexShardTests.testDocStats
  Introduce the dissect library (#32297)
  Security: remove password hash bootstrap check (#32440)
  Move validation to server for put user requests (#32471)
  [ML] Add high level REST client docs for ML put job endpoint (#32843)
  Test: Fix forbidden uses in test framework (#32824)
  Painless: Change fqn_only to no_import (#32817)
  [test] mute testSearchWithSignificantTermsAgg
  Watcher: Remove unused hipchat render method (#32211)
  Watcher: Remove extraneous auth classes (#32300)
  Watcher: migrate PagerDuty v1 events API to v2 API (#32285)
hub-cap added a commit that referenced this pull request Aug 15, 2018
The auth.basic package was an example of a single implementation
interface that leaked into many different classes. In order to clean
this up, the HttpAuth interface, factories, and Registries all were
removed and the single implementation, BasicAuth, was substituted in all
cases. This removes some dependenies between Auth and the Templates,
which can now use static methods on BasicAuth. BasicAuth was also moved
into the http package and all of the other classes were removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants