feat: Kube API Test #5711
feat: Kube API Test #5711
Conversation
|
TODO for myself:
|
| @@ -123,6 +123,7 @@ | |||
| <asm.bundle.version>8.0.1</asm.bundle.version> | |||
| <jackson.bundle.version>${jackson.version}</jackson.bundle.version> | |||
| <slf4j.version>2.0.11</slf4j.version> | |||
| <log4j.version>2.22.0</log4j.version> | |||
There was a problem hiding this comment.
We'll eventually want to unify the logging system
There was a problem hiding this comment.
Note that this is just for tests
There was a problem hiding this comment.
Noted, but the idea is to keep dependencies to the minimum. Anyway, this is not urgent or critical.
| private void setKubernetesClientToField(ExtensionContext extensionContext, | ||
| Field kubeClientField, KubeAPIServer kubeApiServer) { | ||
| try { | ||
| var target = extensionContext.getTestInstance() |
There was a problem hiding this comment.
Note to self:
Once merged create issue to track the creation of a common junit module that provides the tooling to set fields, especially with the consideration for @Nested
| <dependency> | ||
| <groupId>org.eclipse.jetty</groupId> | ||
| <artifactId>jetty-server</artifactId> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Note to self:
Curious about this one. Review current usage, see if can be easily replaced by Vert.x.
There was a problem hiding this comment.
Can try to replace it with Very.x. Maybe a subsequent PR?
There was a problem hiding this comment.
Yes, don't worry, this is just a reminder for me, especially with regards to #5632
| import java.util.Date; | ||
| import java.util.concurrent.locks.ReentrantLock; | ||
|
|
||
| public class CertManager { |
There was a problem hiding this comment.
Note that Vert.x provides this functionality, see SelfSignedCertificate.create
There was a problem hiding this comment.
Are you ok to have a separate PR for that?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| new TrustManager[] { | ||
| new X509ExtendedTrustManager() { | ||
| @Override | ||
| public void checkClientTrusted(X509Certificate[] chain, String authType, |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
| return null; | ||
| } | ||
|
|
||
| public void checkClientTrusted( |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
| final String a_auth_type) { | ||
| } | ||
|
|
||
| public void checkServerTrusted( |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
| final String a_auth_type) { | ||
| } | ||
|
|
||
| public void checkServerTrusted( |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
| } | ||
|
|
||
| @Override | ||
| public void checkClientTrusted(X509Certificate[] chain, String authType, |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
|
|
||
| } | ||
|
|
||
| public void checkServerTrusted( |
Check failure
Code scanning / SonarCloud
Server certificates should be verified during SSL/TLS connections High
| @Test | ||
| @EnableKubeAPIServer | ||
| void simpleTest2() { | ||
| TestUtils.simpleTest(client); |
There was a problem hiding this comment.
Instead of reusing static method for shared tests, do you think making an abstract class for simpleTest and making these classes extend that class would be better? Just like we do in httpclient tests...
There was a problem hiding this comment.
To be honest I don't really like that pattern, when we extending tests. But if everybody agrees it should be done that way in this project, I can change it.
I personally like it better with this "composition" over inheritance approach.
There was a problem hiding this comment.
Although the naming is not really good imo, maybe TestCase / TestCaseUtils might be better.
There was a problem hiding this comment.
I don't like abstract tests either. However, I don't like the usage of this static test methods :).
Anyway, I'm sure we'll find a way to refactor the tests later on, especially if they are all doing the same.
|
|
||
| import java.util.Map; | ||
|
|
||
| public class TestUtils { |
There was a problem hiding this comment.
This class looks very much the same as io.fabric8.kubeapitest.junit.TestUtils . Is it possible to reduce duplication?
There was a problem hiding this comment.
Not sure if there is an elegant way to do this, other than test-jar:
https://stackoverflow.com/questions/7751860/how-do-i-include-a-dependencys-test-jar-into-a-maven-projects-deployment
But personally I prefer code duplication over to this.
...est/core/src/test/java/io/fabric8/kubeapitest/sample/KubernetesMutationHookHandlingTest.java
Show resolved
Hide resolved
| public static int findFreePort() { | ||
| try (ServerSocket socket = new ServerSocket(0)) { | ||
| return socket.getLocalPort(); | ||
| } catch (IOException e) { | ||
|
|
||
| } | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
There was a problem hiding this comment.
ok, borrowed some code from jkube. should be fixed :)
There was a problem hiding this comment.
There's some logic duplication that we'll need to find a way yo unify in the future.
It might also be interesting to mark these modules as (preview) with a non-stable API, just in case we need to modify a few things once we start getting broader feedback.
Other than that, seems good to go for me.
|
|
||
| class CertManagerTest { | ||
|
|
||
| File certsDir = new File("target", "certs"); |
There was a problem hiding this comment.
I might be wrong here but would it be better to create this under a@TempDir rather than in project directory itself?
There was a problem hiding this comment.
good point, that would be better, for sure
junit/kube-api-test/core/src/test/java/io/fabric8/kubeapitest/kubeconfig/KubeConfigTest.java
Show resolved
Hide resolved
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
…junit/KubeAPIServerExtension.java Co-authored-by: Rohan Kumar <rohan.kumar.kanojia@gmail.com>
Co-authored-by: Rohan Kumar <rohan.kumar.kanojia@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
csviri
force-pushed
the
kube-api-test
branch
from
34a88d3
to
bd54524
Compare
Signed-off-by: Attila Mészáros <csviri@gmail.com>
|
np, rebased, the Openshift builds seem to be failing on all PR, not sure if the sonar results are good enough this way. |
Description
Moving and renaming Jenvtest here.
Type of change
test, version modification, documentation, etc.)
Checklist