You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "zachjsh (via GitHub)" <gi...@apache.org> on 2023/03/30 20:18:23 UTC
[GitHub] [druid] zachjsh opened a new pull request, #14003: Allow for Input source security in native task layer
zachjsh opened a new pull request, #14003:
URL: https://github.com/apache/druid/pull/14003
Fixes #13837.
### Description
This change allows for input source type security in the native task layer.
To enable this feature, the user must set the following property to true:
`druid.auth.enableInputSourceSecurity=true`
The default value for this property is false, which will continue the existing functionality of needing authorization to write to the respective datasource.
When this config is enabled, the users will be required to be authorized for the following resource action, in addition to write permission on the respective datasource.
`new ResourceAction(new Resource(ResourceType.EXTERNAL, {INPUT_SOURCE_TYPE}, Action.READ`
where `{INPUT_SOURCE_TYPE}` is the type of the input source being used;, http, inline, s3, etc..
Only tasks that return non-null inputSource types will be considered for authorization checks against the input source type,
otherwise only write permission on the datasource is checked.
TODO: Need to add tests and documentation. Just wanted to get this up here to get some eyes on it.
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157513262
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -188,14 +194,36 @@ public OverlordResource(
public Response taskPost(final Task task, @Context final HttpServletRequest req)
{
final String dataSource = task.getDataSource();
- final ResourceAction resourceAction = new ResourceAction(
- new Resource(dataSource, ResourceType.DATASOURCE),
- Action.WRITE
- );
+ final Set<ResourceAction> resourceActions = new HashSet<>();
+ resourceActions.add(new ResourceAction(new Resource(dataSource, ResourceType.DATASOURCE), Action.WRITE));
+ if (authConfig.isEnableInputSourceSecurity()) {
+ if (task.usesFirehose()) {
+ return Response.status(Response.Status.BAD_REQUEST)
+ .entity(
+ ImmutableMap.of(
+ "error",
+ StringUtils.format(
+ "Input source based security cannot be performed for Task[%s] because it uses firehose."
+ + "Change the tasks configuration, or disable `isEnableInputSourceSecurity`",
+ task.getId()
+ )
+ )
+ )
+ .build();
+ }
+ if (null != task.getInputSourceTypes()) {
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1156401854
##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaIndexTask.java:
##########
@@ -132,6 +136,20 @@ public String getType()
return TYPE;
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return Collections.singleton("kafka");
Review Comment:
Is there a Kafka input source? If so, perhaps use the `TYPE_KEY` constant from that class (adding one if one does not already exist.)
##########
processing/src/main/java/org/apache/druid/data/input/impl/CombiningInputSource.java:
##########
@@ -61,6 +63,19 @@ public CombiningInputSource(
this.delegates = delegates;
}
+ @Override
+ public Set<String> getTypes()
+ {
+ Set<String> types = new HashSet<>();
+ for (InputSource delegate : delegates) {
+ Set<String> delegateTypes = delegate.getTypes();
+ if (null != delegateTypes) {
+ types.addAll(delegateTypes);
+ }
+ }
Review Comment:
The above would be easier if we didn't allow nulls:
```java
Set<String> types = new HashSet<>();
for (InputSource delegate : delegates) {
types.addAll(delegate.getTypes());
}
```
Or, the streaming alternative, if IntelliJ inspections doesn't like the above loop.
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +73,15 @@ default boolean isSuspended()
*/
String getType();
+ @Nullable
+ default Set<String> getInputSourceTypes() {
+ return null;
+ }
+
+ default boolean usesFirehose() {
+ return false;
+ }
Review Comment:
Perhaps add comments to explain these method. Especially the `usesFirehose()` method: I gather that firehose doesn't fit into the input security model? Why not? An explanation will help future readers.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionDistributionTask.java:
##########
@@ -174,6 +176,22 @@ public String getType()
return TYPE;
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
Review Comment:
This code appears again and again. Can it be promoted to a base class?
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/AppenderatorDriverRealtimeIndexTask.java:
##########
@@ -213,6 +215,20 @@ public String getType()
return "index_realtime_appenderator";
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return null;
Review Comment:
Is this the right answer? Or, should it be `ImmutableSet.of()` so it is non-null? Here and below.
##########
processing/src/main/java/org/apache/druid/data/input/impl/InlineInputSource.java:
##########
@@ -48,6 +50,12 @@ public InlineInputSource(@JsonProperty("data") String data)
this.data = data;
}
+ @Override
+ public Set<String> getTypes()
+ {
+ return Collections.singleton(TYPE_KEY);
+ }
Review Comment:
Move to the base class since all input sources supported thus far have only one type. (The catalog and table function stuff depend on this fact.)
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -188,14 +194,36 @@ public OverlordResource(
public Response taskPost(final Task task, @Context final HttpServletRequest req)
{
final String dataSource = task.getDataSource();
- final ResourceAction resourceAction = new ResourceAction(
- new Resource(dataSource, ResourceType.DATASOURCE),
- Action.WRITE
- );
+ final Set<ResourceAction> resourceActions = new HashSet<>();
+ resourceActions.add(new ResourceAction(new Resource(dataSource, ResourceType.DATASOURCE), Action.WRITE));
+ if (authConfig.isEnableInputSourceSecurity()) {
+ if (task.usesFirehose()) {
+ return Response.status(Response.Status.BAD_REQUEST)
+ .entity(
+ ImmutableMap.of(
+ "error",
+ StringUtils.format(
+ "Input source based security cannot be performed for Task[%s] because it uses firehose."
+ + "Change the tasks configuration, or disable `isEnableInputSourceSecurity`",
+ task.getId()
+ )
+ )
+ )
+ .build();
+ }
+ if (null != task.getInputSourceTypes()) {
Review Comment:
Nit: we're computing the input source types twice.
```
Set<String> inputSourceTypes = task.getInputSourceTypes();
if (inputSourceTypes != null) {
...
```
Then, the reason we return a `Set` and not a single key is that some input sources are compound. If we allow the input source type method to return null, then those methods that are compound have to include code to handle null.
But, if we say the method must return a non-null set, then it is easy to combine child input sources without null checks.
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,10 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes()
+ {
+ return null;
+ }
Review Comment:
Would be good to add a comment explaining why we return a set, not a single key. Also, see the note above regarding `null` vs. and empty set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1153747885
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java:
##########
@@ -138,6 +140,15 @@ default int getPriority()
*/
String getDataSource();
+ @Nullable
+ default String getInputSourceType() {
Review Comment:
Maybe should add these functions to a different interface, with non-default, and have the tasks that are now implementing properly, implement that interface, so its easier to know which tasks are properly returning this information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157781418
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +74,25 @@ default boolean isSuspended()
*/
String getType();
+ /**
+ * @return The types of {@link org.apache.druid.data.input.InputSource} that the task uses. Empty set is returned if
+ * the task does not use any. Users can be given permission to access particular types of
+ * input sources but not others, using the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config.
+ */
+ default Set<String> getInputSourceTypes() {
+ return ImmutableSet.of();
+ }
+
+ /**
+ * @return Whether the task uses {@link org.apache.druid.data.input.Firehose}. If the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config is
+ * enabled, then tasks that use firehose cannot be used.
+ */
+ default boolean usesFirehose() {
Review Comment:
We should change this to what callers care about. The caller isn't interested in whether a task uses firehoses: it's interested in whether the `getInputSourceTypes` method can be used for authorization. (Consider a case where a task doesn't use firehoses, but still also doesn't support input source authorization.)
The default is also problematic: security stuff must always fail-secure. Doing `return false` here fails insecure: it means that a task that doesn't implement any of this stuff would be allowed. So we should flip that. Putting these together: I'd consider changing this to `boolean canUseInputSourceTypeAuthorization()` and having the default be `return false`.
Or, another option would be eliminating this method, and having `getInputSourceTypes()` (or `getInputSourceResources()`) throw an exception if the task doesn't support input source authorization. The default implementation would need to throw that exception.
(Same comment for `Task`, btw.)
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,10 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes()
+ {
+ return null;
+ }
Review Comment:
It's better here to return a `Set<Resource>` rather than `Set<String>`. It makes it clearer that this is used for security purposes, since `Resource` is a security-specific thing.
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +74,25 @@ default boolean isSuspended()
*/
String getType();
+ /**
+ * @return The types of {@link org.apache.druid.data.input.InputSource} that the task uses. Empty set is returned if
+ * the task does not use any. Users can be given permission to access particular types of
+ * input sources but not others, using the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config.
+ */
+ default Set<String> getInputSourceTypes() {
Review Comment:
Similar to the comment on `InputSource`: it's better for this to be `Set<Resource>`, so it's clear it's security-related. Method name would be `getInputSourceResources()` in that case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157878873
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java:
##########
@@ -270,6 +275,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4617)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionDistributionTask.java:
##########
@@ -174,6 +183,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4619)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialRangeSegmentGenerateTask.java:
##########
@@ -148,6 +156,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4621)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/LegacySinglePhaseSubTask.java:
##########
@@ -56,4 +65,20 @@
{
return SinglePhaseSubTask.OLD_TYPE_NAME;
}
+
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4616)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/SinglePhaseSubTask.java:
##########
@@ -190,6 +197,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4622)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionCardinalityTask.java:
##########
@@ -133,6 +142,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4618)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialHashSegmentGenerateTask.java:
##########
@@ -131,6 +138,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4620)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157872056
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,10 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes()
+ {
+ return null;
+ }
Review Comment:
`ResourceAction` isn't available to InputSources at the moment. I can add a dependency. Ok to do this? Not sure if it will create a dependency cycle
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1153748436
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,9 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes() {
Review Comment:
Maybe should add this function to a different interface, with non-default, and have the tasks that are now implementing properly, implement that interface, so its easier to know which tasks are properly returning this information
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java:
##########
@@ -138,6 +140,15 @@ default int getPriority()
*/
String getDataSource();
+ @Nullable
+ default String getInputSourceType() {
Review Comment:
Maybe should add these functions to a different interface, with non-default, and have the tasks that are now implementing properly, implement that interface, so its easier to know which tasks are properly returning this information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157513526
##########
processing/src/main/java/org/apache/druid/data/input/impl/CombiningInputSource.java:
##########
@@ -61,6 +63,19 @@ public CombiningInputSource(
this.delegates = delegates;
}
+ @Override
+ public Set<String> getTypes()
+ {
+ Set<String> types = new HashSet<>();
+ for (InputSource delegate : delegates) {
+ Set<String> delegateTypes = delegate.getTypes();
+ if (null != delegateTypes) {
+ types.addAll(delegateTypes);
+ }
+ }
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157848276
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -1086,6 +1102,27 @@
}
}
+ @VisibleForTesting
+ Set<ResourceAction> getNeededResourceActionsForTask(Task task) throws IAE
+ {
+ final String dataSource = task.getDataSource();
+ final Set<ResourceAction> resourceActions = new HashSet<>();
+ resourceActions.add(new ResourceAction(new Resource(dataSource, ResourceType.DATASOURCE), Action.WRITE));
+ if (authConfig.isEnableInputSourceSecurity()) {
+ if (task.usesFirehose()) {
+ throw new IAE(StringUtils.format(
+ "Input source based security cannot be performed for Task[%s] because it uses firehose."
+ + "Change the tasks configuration, or disable `isEnableInputSourceSecurity`",
Review Comment:
## Missing space in string literal
This string appears to be missing a space after 'firehose.'.
[Show more details](https://github.com/apache/druid/security/code-scanning/4607)
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopIndexTaskTest.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.task;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.indexer.HadoopIOConfig;
+import org.apache.druid.indexer.HadoopIngestionSpec;
+import org.apache.druid.jackson.DefaultObjectMapper;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.query.aggregation.AggregatorFactory;
+import org.apache.druid.segment.indexing.DataSchema;
+import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec;
+import org.apache.druid.server.security.AuthTestUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+
+public class HadoopIndexTaskTest
+{
+ private final ObjectMapper jsonMapper = new DefaultObjectMapper();
+ @Test
+ public void testCorrectInputSourceTypes() throws Exception
+ {
+ final HadoopIndexTask task = new HadoopIndexTask(
+ null,
+ new HadoopIngestionSpec(
+ new DataSchema(
+ "foo", null, new AggregatorFactory[0], new UniformGranularitySpec(
+ Granularities.DAY,
+ null,
+ ImmutableList.of(Intervals.of("2010-01-01/P1D"))
+ ),
+ null,
+ jsonMapper
+ ), new HadoopIOConfig(ImmutableMap.of("paths", "bar"), null, null), null
Review Comment:
## Deprecated method or constructor invocation
Invoking [DataSchema.DataSchema](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4608)
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopIndexTaskTest.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.task;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.indexer.HadoopIOConfig;
+import org.apache.druid.indexer.HadoopIngestionSpec;
+import org.apache.druid.jackson.DefaultObjectMapper;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.query.aggregation.AggregatorFactory;
+import org.apache.druid.segment.indexing.DataSchema;
+import org.apache.druid.segment.indexing.granularity.UniformGranularitySpec;
+import org.apache.druid.server.security.AuthTestUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+
+public class HadoopIndexTaskTest
+{
+ private final ObjectMapper jsonMapper = new DefaultObjectMapper();
+ @Test
+ public void testCorrectInputSourceTypes() throws Exception
+ {
+ final HadoopIndexTask task = new HadoopIndexTask(
+ null,
+ new HadoopIngestionSpec(
+ new DataSchema(
+ "foo", null, new AggregatorFactory[0], new UniformGranularitySpec(
+ Granularities.DAY,
+ null,
+ ImmutableList.of(Intervals.of("2010-01-01/P1D"))
+ ),
+ null,
+ jsonMapper
+ ), new HadoopIOConfig(ImmutableMap.of("paths", "bar"), null, null), null
+ ),
+ null,
+ null,
+ "blah",
+ jsonMapper,
+ null,
+ AuthTestUtils.TEST_AUTHORIZER_MAPPER,
+ null
+ );
+
+ Assert.assertEquals(Collections.singleton("hadoop"), task.getInputSourceTypes());
+ }
+
+ @Test
+ public void testDoesntUseFirehose() throws Exception
+ {
+ final HadoopIndexTask task = new HadoopIndexTask(
+ null,
+ new HadoopIngestionSpec(
+ new DataSchema(
+ "foo", null, new AggregatorFactory[0], new UniformGranularitySpec(
+ Granularities.DAY,
+ null,
+ ImmutableList.of(Intervals.of("2010-01-01/P1D"))
+ ),
+ null,
+ jsonMapper
+ ), new HadoopIOConfig(ImmutableMap.of("paths", "bar"), null, null), null
Review Comment:
## Deprecated method or constructor invocation
Invoking [DataSchema.DataSchema](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4609)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1153748436
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,9 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes() {
Review Comment:
Maybe should add this function to a different interface, with non-default, and have the tasks that are now implementing properly, implement that interface, so its easier to know which tasks are properly returning this information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1158661902
##########
processing/src/main/java/org/apache/druid/data/input/InputSource.java:
##########
@@ -87,4 +88,10 @@ InputSourceReader reader(
@Nullable InputFormat inputFormat,
File temporaryDirectory
);
+
+ @Nullable
+ default Set<String> getTypes()
+ {
+ return null;
+ }
Review Comment:
I tried this and unfortunately it added a cyclic dependency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh merged pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh merged PR #14003:
URL: https://github.com/apache/druid/pull/14003
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157859165
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +74,25 @@ default boolean isSuspended()
*/
String getType();
+ /**
+ * @return The types of {@link org.apache.druid.data.input.InputSource} that the task uses. Empty set is returned if
+ * the task does not use any. Users can be given permission to access particular types of
+ * input sources but not others, using the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config.
+ */
+ default Set<String> getInputSourceTypes() {
Review Comment:
Good suggestion, fixed.
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +74,25 @@ default boolean isSuspended()
*/
String getType();
+ /**
+ * @return The types of {@link org.apache.druid.data.input.InputSource} that the task uses. Empty set is returned if
+ * the task does not use any. Users can be given permission to access particular types of
+ * input sources but not others, using the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config.
+ */
+ default Set<String> getInputSourceTypes() {
+ return ImmutableSet.of();
+ }
+
+ /**
+ * @return Whether the task uses {@link org.apache.druid.data.input.Firehose}. If the
+ * {@link org.apache.druid.server.security.AuthConfig#enableInputSourceSecurity} config is
+ * enabled, then tasks that use firehose cannot be used.
+ */
+ default boolean usesFirehose() {
Review Comment:
Good suggestion, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157980442
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/LegacySinglePhaseSubTask.java:
##########
@@ -56,4 +65,20 @@
{
return SinglePhaseSubTask.OLD_TYPE_NAME;
}
+
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4720)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionCardinalityTask.java:
##########
@@ -133,6 +142,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4722)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialHashSegmentGenerateTask.java:
##########
@@ -131,6 +138,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4724)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java:
##########
@@ -270,6 +275,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4721)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialRangeSegmentGenerateTask.java:
##########
@@ -148,6 +156,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4725)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/SinglePhaseSubTask.java:
##########
@@ -190,6 +197,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4726)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionDistributionTask.java:
##########
@@ -174,6 +183,22 @@
return TYPE;
}
+ @Nonnull
+ @JsonIgnore
+ @Override
+ public Set<ResourceAction> getInputSourceResources()
+ {
+ if (getIngestionSchema().getIOConfig().getFirehoseFactory() != null) {
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4723)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157520353
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionDistributionTask.java:
##########
@@ -174,6 +176,22 @@ public String getType()
return TYPE;
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
Review Comment:
These task definitions are a little brittle, think this would be a little risky to do at this time.
##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java:
##########
@@ -71,6 +73,15 @@ default boolean isSuspended()
*/
String getType();
+ @Nullable
+ default Set<String> getInputSourceTypes() {
+ return null;
+ }
+
+ default boolean usesFirehose() {
+ return false;
+ }
Review Comment:
added comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] zachjsh commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "zachjsh (via GitHub)" <gi...@apache.org>.
zachjsh commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1157512300
##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaIndexTask.java:
##########
@@ -132,6 +136,20 @@ public String getType()
return TYPE;
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return Collections.singleton("kafka");
Review Comment:
added
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/AppenderatorDriverRealtimeIndexTask.java:
##########
@@ -213,6 +215,20 @@ public String getType()
return "index_realtime_appenderator";
}
+ @JsonIgnore
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return null;
Review Comment:
good point, changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #14003: Allow for Input source security in native task layer
Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #14003:
URL: https://github.com/apache/druid/pull/14003#discussion_r1153859882
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialHashSegmentGenerateTask.java:
##########
@@ -131,6 +132,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
+ getIngestionSchema().getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return getIngestionSchema().getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4505)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialRangeSegmentGenerateTask.java:
##########
@@ -148,6 +149,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
+ getIngestionSchema().getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return getIngestionSchema().getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4506)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/SinglePhaseSubTask.java:
##########
@@ -190,6 +190,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return ingestionSchema.getIOConfig().getInputSource() != null ?
+ ingestionSchema.getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return ingestionSchema.getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4507)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/LegacySinglePhaseSubTask.java:
##########
@@ -56,4 +57,18 @@
{
return SinglePhaseSubTask.OLD_TYPE_NAME;
}
+
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
+ getIngestionSchema().getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return getIngestionSchema().getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4501)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionCardinalityTask.java:
##########
@@ -133,6 +134,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
+ getIngestionSchema().getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return getIngestionSchema().getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4503)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/PartialDimensionDistributionTask.java:
##########
@@ -174,6 +175,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return getIngestionSchema().getIOConfig().getInputSource() != null ?
+ getIngestionSchema().getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return getIngestionSchema().getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4504)
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java:
##########
@@ -270,6 +270,20 @@
return TYPE;
}
+ @Nullable
+ @Override
+ public Set<String> getInputSourceTypes()
+ {
+ return ingestionSchema.getIOConfig().getInputSource() != null ?
+ ingestionSchema.getIOConfig().getInputSource().getTypes() :
+ null;
+ }
+
+ @Override
+ public boolean usesFirehose() {
+ return ingestionSchema.getIOConfig().getFirehoseFactory() != null;
Review Comment:
## Deprecated method or constructor invocation
Invoking [IndexIOConfig.getFirehoseFactory](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/4502)
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java:
##########
@@ -188,14 +194,36 @@
public Response taskPost(final Task task, @Context final HttpServletRequest req)
{
final String dataSource = task.getDataSource();
- final ResourceAction resourceAction = new ResourceAction(
- new Resource(dataSource, ResourceType.DATASOURCE),
- Action.WRITE
- );
+ final Set<ResourceAction> resourceActions = new HashSet<>();
+ resourceActions.add(new ResourceAction(new Resource(dataSource, ResourceType.DATASOURCE), Action.WRITE));
+ if (authConfig.isEnableInputSourceSecurity()) {
+ if (task.usesFirehose()) {
+ return Response.status(Response.Status.BAD_REQUEST)
+ .entity(
+ ImmutableMap.of(
+ "error",
+ StringUtils.format(
+ "Input source based security cannot be performed for Task[%s] because it uses firehose."
+ + "Change the tasks configuration, or disable `isEnableInputSourceSecurity`",
Review Comment:
## Missing space in string literal
This string appears to be missing a space after 'firehose.'.
[Show more details](https://github.com/apache/druid/security/code-scanning/4500)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org