-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parsing kube config file without reflection. Fixes #1591 #1594
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
...es-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/AuthInfoMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.AuthInfo; | ||
|
||
public class AuthInfoMapper extends PropertyMapper<AuthInfo> { | ||
|
||
private final NamedExtensionsMapper namedExtensionsMapper = new NamedExtensionsMapper(); | ||
private final AuthProviderMapper authProviderConfigMapper = new AuthProviderMapper(); | ||
private final ExecConfigMapper execConfigMapper = new ExecConfigMapper(); | ||
|
||
@SuppressWarnings("unchecked") | ||
@Override | ||
public AuthInfo map(Map<String, Object> mappedBean) { | ||
final AuthInfo authInfo = new AuthInfo(); | ||
authInfo.setAs(get("as", mappedBean)); | ||
authInfo.setAsGroups(getList("as-groups", mappedBean)); | ||
authInfo.setAsUserExtra(get("as-user-extra", mappedBean, Map.class)); | ||
authInfo.setAuthProvider(get("auth-provider", mappedBean, authProviderConfigMapper)); | ||
authInfo.setClientCertificate(get("client-certificate", mappedBean)); | ||
authInfo.setClientCertificateData(get("client-certificate-data", mappedBean)); | ||
authInfo.setClientKey(get("client-key", mappedBean)); | ||
authInfo.setClientKeyData(get("client-key-data", mappedBean)); | ||
authInfo.setExec(get("exec", mappedBean, execConfigMapper)); | ||
authInfo.setExtensions(getList("extensions", mappedBean, namedExtensionsMapper)); | ||
authInfo.setPassword(get("password", mappedBean)); | ||
authInfo.setToken(get("token", mappedBean)); | ||
authInfo.setTokenFile(get("tokenFile", mappedBean)); | ||
authInfo.setUsername(get("username", mappedBean)); | ||
return authInfo; | ||
} | ||
|
||
} |
33 changes: 33 additions & 0 deletions
33
...lient/src/main/java/io/fabric8/kubernetes/client/internal/mappers/AuthProviderMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.AuthProviderConfig; | ||
|
||
@SuppressWarnings("unchecked") | ||
public class AuthProviderMapper extends PropertyMapper<AuthProviderConfig> { | ||
|
||
@Override | ||
public AuthProviderConfig map(Map<String, Object> mappedBean) { | ||
final AuthProviderConfig bean = new AuthProviderConfig(); | ||
bean.setConfig(get("config", mappedBean, Map.class)); | ||
bean.setName(get("name", mappedBean)); | ||
return bean; | ||
} | ||
|
||
} |
37 changes: 37 additions & 0 deletions
37
...tes-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/ClusterMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.Cluster; | ||
|
||
public class ClusterMapper extends PropertyMapper<Cluster> { | ||
|
||
private final NamedExtensionsMapper namedExtensionsMapper = new NamedExtensionsMapper(); | ||
|
||
@Override | ||
public Cluster map(Map<String, Object> mappedBean) { | ||
final Cluster cluster = new Cluster(); | ||
cluster.setCertificateAuthority(get("certificate-authority", mappedBean, String.class)); | ||
cluster.setCertificateAuthorityData(get("certificate-authority-data", mappedBean, String.class)); | ||
cluster.setExtensions(getList("extensions", mappedBean, namedExtensionsMapper)); | ||
cluster.setInsecureSkipTlsVerify(get("insecure-skip-tls-verify", mappedBean, Boolean.class)); | ||
cluster.setServer(get("server", mappedBean, String.class)); | ||
return cluster; | ||
} | ||
|
||
} |
44 changes: 44 additions & 0 deletions
44
...etes-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/ConfigMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.Config; | ||
|
||
public class ConfigMapper extends PropertyMapper<Config> { | ||
|
||
private final NamedClusterMapper namedClusterMapper = new NamedClusterMapper(); | ||
private final NamedContextMapper namedContextMapper = new NamedContextMapper(); | ||
private final NamedExtensionsMapper namedExtensionsMapper = new NamedExtensionsMapper(); | ||
private final PreferencesMapper preferencesMapper = new PreferencesMapper(); | ||
private final NamedAuthInfoMapper namedAuthInfoMapper = new NamedAuthInfoMapper(); | ||
|
||
@Override | ||
public Config map(Map<String, Object> mappedBean) { | ||
final Config config = new Config(); | ||
config.setApiVersion(get("apiVersion", mappedBean, String.class)); | ||
config.setClusters(getList("clusters", mappedBean, namedClusterMapper)); | ||
config.setContexts(getList("contexts", mappedBean, namedContextMapper)); | ||
config.setCurrentContext(get("current-context", mappedBean, String.class)); | ||
config.setExtensions(getList("extensions", mappedBean, namedExtensionsMapper)); | ||
config.setKind(get("kind", mappedBean, String.class)); | ||
config.setPreferences(get("preferences", mappedBean, preferencesMapper)); | ||
config.setUsers(getList("users", mappedBean, namedAuthInfoMapper)); | ||
return config; | ||
} | ||
|
||
} |
36 changes: 36 additions & 0 deletions
36
...tes-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/ContextMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.Context; | ||
|
||
public class ContextMapper extends PropertyMapper<Context> { | ||
|
||
private final NamedExtensionsMapper namedExtensionsMapper = new NamedExtensionsMapper(); | ||
|
||
@Override | ||
public Context map(Map<String, Object> mappedBean) { | ||
final Context bean = new Context(); | ||
bean.setCluster(get("cluster", mappedBean, String.class)); | ||
bean.setUser(get("user", mappedBean, String.class)); | ||
bean.setNamespace(get("namespace", mappedBean, String.class)); | ||
bean.setExtensions(getList("extensions", mappedBean, namedExtensionsMapper)); | ||
return bean; | ||
} | ||
|
||
} |
36 changes: 36 additions & 0 deletions
36
...-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/ExecConfigMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.ExecConfig; | ||
|
||
public class ExecConfigMapper extends PropertyMapper<ExecConfig> { | ||
|
||
private final ExecEnvVarMapper execEnvVarMapper = new ExecEnvVarMapper(); | ||
|
||
@Override | ||
public ExecConfig map(Map<String, Object> mappedBean) { | ||
final ExecConfig bean = new ExecConfig(); | ||
bean.setApiVersion(get("apiVersion", mappedBean)); | ||
bean.setArgs(getList("args", mappedBean)); | ||
bean.setCommand(get("command", mappedBean)); | ||
bean.setEnv(getList("env", mappedBean, execEnvVarMapper)); | ||
return bean; | ||
} | ||
|
||
} |
32 changes: 32 additions & 0 deletions
32
...-client/src/main/java/io/fabric8/kubernetes/client/internal/mappers/ExecEnvVarMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.ExecEnvVar; | ||
|
||
public class ExecEnvVarMapper extends PropertyMapper<ExecEnvVar> { | ||
|
||
@Override | ||
public ExecEnvVar map(Map<String, Object> mappedBean) { | ||
final ExecEnvVar bean = new ExecEnvVar(); | ||
bean.setName(get("name", mappedBean)); | ||
bean.setValue(get("value", mappedBean)); | ||
return bean; | ||
} | ||
|
||
} |
34 changes: 34 additions & 0 deletions
34
...ient/src/main/java/io/fabric8/kubernetes/client/internal/mappers/NamedAuthInfoMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.NamedAuthInfo; | ||
|
||
public class NamedAuthInfoMapper extends PropertyMapper<NamedAuthInfo> { | ||
|
||
private final AuthInfoMapper authInfoMapper = new AuthInfoMapper(); | ||
|
||
@Override | ||
public NamedAuthInfo map(Map<String, Object> mappedBean) { | ||
final NamedAuthInfo bean = new NamedAuthInfo(); | ||
bean.setName(get("name", mappedBean, String.class)); | ||
bean.setUser(get("user", mappedBean, authInfoMapper)); | ||
return bean; | ||
} | ||
|
||
} |
33 changes: 33 additions & 0 deletions
33
...lient/src/main/java/io/fabric8/kubernetes/client/internal/mappers/NamedClusterMapper.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/** | ||
* Copyright (C) 2015 Red Hat, Inc. | ||
* | ||
* Licensed 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 io.fabric8.kubernetes.client.internal.mappers; | ||
|
||
import java.util.Map; | ||
|
||
import io.fabric8.kubernetes.api.model.NamedCluster; | ||
|
||
public class NamedClusterMapper extends PropertyMapper<NamedCluster> { | ||
|
||
private final ClusterMapper clusterMapper = new ClusterMapper(); | ||
|
||
@Override | ||
public NamedCluster map(Map<String, Object> mappedBean) { | ||
final NamedCluster bean = new NamedCluster(); | ||
bean.setName(get("name", mappedBean, String.class)); | ||
bean.setCluster(get("cluster", mappedBean, clusterMapper)); | ||
return bean; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you replace jackson with a custom snakeyaml + custom deserializers?
Is this really needed?
Why is jackson an issue only on
Config
and not when desrializing responses from the api server?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @iocanel! I replaced only the
Config
model because this way we can leverage from the configuration and authentication features from the client without using reflection. The idea is to replace with a custom mapper the responses as well.In our project, having the client handle the authentication features, we can leverage from the
OkHttpClient
to query the API and deserialize using another options (like json path for example).I see three options to handle this deserialization issue when working with Quarkus in a native image:
This PR tackles the option 3 since the use case is only to parse the kube yaml files and it seemed a simple approach at first. Those mappers could be generated during build time, like the way you guys are doing with the domain model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, my undersntanding is that long term we will go with option
3
for everything.Untill, then can't we handle
Config
using1
?I want to avoid introducing a mapping mechanism just for handling a single class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we perhaps just add a GraalVM json file for registering reflection with GraalVM as @iocanel mentions? Wouldn't that be an easier first step?
Also, that file could contain other GraalVM specific configurations, could it not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iocanel a long term option should be
3
if we decide to drop reflection usage at all. You're right to deny introducing a custom mapper for a single class, I'd say the same. This is just a proposal for a mapper that we could build for the entire client. I could work on it.@geoand manually registering the entire domain model is the easiest solution and it's the approach that you took for the quarkusio/quarkus#2910 PR, right? Yeah, I guess that this option is a great first step and we can go ahead with it! For this PR, though, I'd say to introduce the option
3
for the entire client.So, the question is: should we create our custom (generated) mapper for the long term or we stick with manual reflection registration provided by Quarkus/SubtrateVM? Having the custom mapper won't exclude the Quarkus extension, since there's still the unsupported feature in subtrateVM regarding the EC Keys with Bouncy Castle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that making incremental improvements on reducing reflection in this project will be great down the line. That said, I think @iocanel has some thoughts on how to make that easier based on some other he has done.
The Quarkus PR does register everything in the Kubernetes Model for reflection, but with a twist :). It doesn't force everything to end up in the native binary, but instructs GraalVM to include all methods and fields for only those model classes that are actually used in the application that includes the it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess that we can close this PR since @iocanel is already working on something? I worked around the
Config
parsing stuff just settingKUBERNETES_AUTH_TRYKUBECONFIG
tofalse
. And then mapped the JSON response as simpleMap
s to avoid reflection of the domain classes. Worked, but now I guess with the extension I'll drop this work around.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's what and see what the man himself will say :).
I know the work in question was done on another project so I am not sure if it can be of immediate use here.
In any case, thanks a lot of looking into this!