You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "emiliosetiadarma (via GitHub)" <gi...@apache.org> on 2023/06/28 18:40:44 UTC

[GitHub] [nifi] emiliosetiadarma opened a new pull request, #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

emiliosetiadarma opened a new pull request, #7446:
URL: https://github.com/apache/nifi/pull/7446

   … reloads SSL context, changed TrustStoreScanner to extend the AbstractStoreScanner, and implemented unit tests
   
   <!-- 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. -->
   
   # Summary
   
   [NIFI-11536](https://issues.apache.org/jira/browse/NIFI-11536)
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
   
   ### Pull Request Tracking
   
   - [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [x] Pull Request based on current revision of the `main` branch
   - [x] Pull Request refers to a feature branch with one commit containing changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request creation.
   
   ### Build
   
   - [x] Build completed using `mvn clean install -P contrib-check`
     - [x] JDK 17
   
   ### Licensing
   
   - [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] emiliosetiadarma commented on pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "emiliosetiadarma (via GitHub)" <gi...@apache.org>.
emiliosetiadarma commented on PR #7446:
URL: https://github.com/apache/nifi/pull/7446#issuecomment-1612201484

   I made the changes while still preserving `KeyStoreScanner` and `TrustStoreScanner` as the derived classes for two reasons. 
   
   1. So the logger is class-specific
   2. So the `server` would have beans with distinct types. Having all have the same type would cause some existing tests to fail since it uses `server.getBean`.
   
   Some thoughts: I was wondering if we were to have just one class whether it's a good idea to have both a keystore scanner and truststore scanner in the same class. One advantage this would have is if both keystore and truststore changed, then the `SSLContext` would be reloaded only once, as opposed to having two scanners that reloaded it twice. 


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] emiliosetiadarma commented on a diff in pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "emiliosetiadarma (via GitHub)" <gi...@apache.org>.
emiliosetiadarma commented on code in PR #7446:
URL: https://github.com/apache/nifi/pull/7446#discussion_r1245806680


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();
+    protected abstract Resource getResource();

Review Comment:
   Making the changes



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;

Review Comment:
   Making the changes



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] emiliosetiadarma commented on pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "emiliosetiadarma (via GitHub)" <gi...@apache.org>.
emiliosetiadarma commented on PR #7446:
URL: https://github.com/apache/nifi/pull/7446#issuecomment-1612132072

   Thanks @exceptionfactory for the quick review! Will address the 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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #7446:
URL: https://github.com/apache/nifi/pull/7446#discussion_r1245778923


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();

Review Comment:
   Recommend using a constructor argument to provide the File Type, instead of an abstract method.



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();
+    protected abstract Resource getResource();
+    protected abstract Logger getLogger();

Review Comment:
   Instead of making `getLogger()` abstract, the Logger can be defined as an instance variable with `getClass()` so that concrete classes will have distinct loggers.



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;

Review Comment:
   Although Jetty uses the `_scanner` name, recommend removing the `_` character to follow Apache NiFi project conventions.



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();
+    protected abstract Resource getResource();

Review Comment:
   The `Resource` could also be provided as a constructor argument, eliminating the need for abstract methods. With those changes, the same class can be used without the need for subclasses.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] emiliosetiadarma commented on a diff in pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "emiliosetiadarma (via GitHub)" <gi...@apache.org>.
emiliosetiadarma commented on code in PR #7446:
URL: https://github.com/apache/nifi/pull/7446#discussion_r1245806477


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();

Review Comment:
   Made the constructor argument take in a fileType argument



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory closed pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory closed pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…
URL: https://github.com/apache/nifi/pull/7446


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] emiliosetiadarma commented on a diff in pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "emiliosetiadarma (via GitHub)" <gi...@apache.org>.
emiliosetiadarma commented on code in PR #7446:
URL: https://github.com/apache/nifi/pull/7446#discussion_r1245806247


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/util/AbstractStoreScanner.java:
##########
@@ -0,0 +1,175 @@
+/*
+ * 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.nifi.web.server.util;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.security.util.TlsConfiguration;
+import org.apache.nifi.security.util.TlsException;
+import org.eclipse.jetty.util.Scanner;
+import org.eclipse.jetty.util.annotation.ManagedAttribute;
+import org.eclipse.jetty.util.annotation.ManagedOperation;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
+
+public abstract class AbstractStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
+    private final SslContextFactory sslContextFactory;
+    private final TlsConfiguration tlsConfiguration;
+    private final File file;
+    private final Scanner _scanner;
+
+    protected AbstractStoreScanner(final SslContextFactory sslContextFactory,
+                                   final TlsConfiguration tlsConfiguration) {
+        this.sslContextFactory = sslContextFactory;
+        this.tlsConfiguration = tlsConfiguration;
+        try {
+            final Resource resource = getResource();
+            File monitoredFile = resource.getFile();
+            if (monitoredFile == null || !monitoredFile.exists()) {
+                throw new IllegalArgumentException(String.format("%s file does not exist", getFileType()));
+            }
+            if (monitoredFile.isDirectory()) {
+                throw new IllegalArgumentException(String.format("expected %s file not directory", getFileType()));
+            }
+
+            if (resource.getAlias() != null) {
+                // this resource has an alias, use the alias, as that's what's returned in the Scanner
+                monitoredFile = new File(resource.getAlias());
+            }
+
+            file = monitoredFile;
+            if (getLogger().isDebugEnabled()) {
+                getLogger().debug("Monitored {} file: {}", getFileType(), monitoredFile);
+            }
+        } catch (final IOException e) {
+            throw new IllegalArgumentException(String.format("could not obtain %s file", getFileType()), e);
+        }
+
+        File parentFile = file.getParentFile();
+        if (!parentFile.exists() || !parentFile.isDirectory()) {
+            throw new IllegalArgumentException(String.format("error obtaining %s dir", getFileType()));
+        }
+
+        _scanner = new Scanner();
+        _scanner.setScanDirs(Collections.singletonList(parentFile));
+        _scanner.setScanInterval(1);
+        _scanner.setReportDirs(false);
+        _scanner.setReportExistingFilesOnStartup(false);
+        _scanner.setScanDepth(1);
+        _scanner.addListener(this);
+        addBean(_scanner);
+    }
+
+    @Override
+    public void fileAdded(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("added {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileChanged(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("changed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @Override
+    public void fileRemoved(final String filename) {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("removed {}", filename);
+        }
+
+        if (this.file.toPath().toString().equals(filename)) {
+            this.reload();
+        }
+
+    }
+
+    @ManagedOperation(
+            value = "Scan for changes in the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void scan() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("scanning");
+        }
+
+        this._scanner.scan();
+        this._scanner.scan();
+    }
+
+    @ManagedOperation(
+            value = "Reload the SSL Keystore/Truststore",
+            impact = "ACTION"
+    )
+    public void reload() {
+        if (getLogger().isDebugEnabled()) {
+            getLogger().debug("reloading {} file {}", getFileType(), this.file);
+        }
+
+        try {
+            this.sslContextFactory.reload((scf) -> scf.setSslContext(createSSLContext(tlsConfiguration)));
+        } catch (final Throwable t) {
+            getLogger().warn("{} Reload Failed", StringUtils.capitalize(getFileType()), t);
+        }
+
+    }
+
+    @ManagedAttribute("scanning interval to detect changes which need reloaded")
+    public int getScanInterval() {
+        return this._scanner.getScanInterval();
+    }
+
+    public void setScanInterval(int scanInterval) {
+        this._scanner.setScanInterval(scanInterval);
+    }
+
+    protected SslContextFactory getSslContextFactory() {
+        return sslContextFactory;
+    }
+
+    protected abstract String getFileType();
+    protected abstract Resource getResource();
+    protected abstract Logger getLogger();

Review Comment:
   Got it. Will change it to use `getClass()`



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on pull request #7446: NIFI-11536: implemented AbstractStoreScanner and KeyStoreScanner that…

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on PR #7446:
URL: https://github.com/apache/nifi/pull/7446#issuecomment-1612209337

   > I made the changes while still preserving `KeyStoreScanner` and `TrustStoreScanner` as the derived classes for two reasons.
   > 
   >     1. So the logger is class-specific
   
   It is not necessary to define a class-specific logger, although maintaining two separate class implementations might be useful based on the second concern.
   
   > 
   >     2. So the `server` would have beans with distinct types. Having all have the same type would cause some existing tests to fail since it uses `server.getBean`.
   
   That is a good point regarding `addBean` and `getBean`. With that in mind, it does seem reasonable to maintain two separate classes.
   
   > 
   > 
   > Some thoughts: I was wondering if we were to have just one class whether it's a good idea to have both a keystore scanner and truststore scanner in the same class. One advantage this would have is if both keystore and truststore changed, then the `SSLContext` would be reloaded only once, as opposed to having two scanners that reloaded it twice.
   
   Although having one class might be worth considering, in general the keystore and truststore should change independently, so keeping two separate classes sounds good.
   
   Thanks for considering the options.
   


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org