-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support for Node-Specific Restrictions in Fluid #4508
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| {{- if and (not .Values.csi.useNodeAuthorization) (semverCompare ">=1.30.0-0" .Capabilities.KubeVersion.Version) -}} | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicy | ||
| metadata: | ||
| name: "fluid-csi-node-policy" | ||
| spec: | ||
| failurePolicy: Fail | ||
| matchConstraints: | ||
| resourceRules: | ||
| - apiGroups: [""] | ||
| apiVersions: ["v1"] | ||
| # supported values: "*", "CONNECT", "CREATE", "DELETE", "UPDATE" | ||
| operations: ["UPDATE"] | ||
| resources: ["nodes"] | ||
| matchConditions: | ||
| # only fluid-csi request will be checked. | ||
| - name: isRestrictedUser | ||
| expression: request.userInfo.username == "system:serviceaccount:fluid-system:fluid-csi" | ||
| variables: | ||
| - name: userNodeName | ||
| expression: >- | ||
| request.userInfo.extra[?'authentication.kubernetes.io/node-name'][0].orValue('') | ||
| - name: objectNodeName | ||
| expression: >- | ||
| object.?metadata.name.orValue('') | ||
| validations: | ||
| - expression: "variables.userNodeName != ''" | ||
| message: "userNodeName is empty, user token does not contain node name." | ||
| - expression: "variables.objectNodeName == variables.userNodeName" | ||
| messageExpression: >- | ||
| "objectNodeName '" + variables.objectNodeName + "' is not equal to userNodeName '" + variables.userNodeName + "'" | ||
| {{- end }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| {{- if and (not .Values.csi.useNodeAuthorization) (semverCompare ">=1.30.0-0" .Capabilities.KubeVersion.Version) -}} | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicyBinding | ||
| metadata: | ||
| name: "fluid-csi-node-policy-binding" | ||
| spec: | ||
| policyName: "fluid-csi-node-policy" | ||
| validationActions: [Deny] | ||
| {{- end }} |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||||||
| /* | ||||||||||||||
| Copyright 2025 The Fluid Authors. | ||||||||||||||
| 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 plugins | ||||||||||||||
|
|
||||||||||||||
| import ( | ||||||||||||||
| "context" | ||||||||||||||
| corev1 "k8s.io/api/core/v1" | ||||||||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||
| "k8s.io/apimachinery/pkg/types" | ||||||||||||||
| "k8s.io/client-go/kubernetes" | ||||||||||||||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| type NodeAuthorizedClient interface { | ||||||||||||||
| Get(nodeName string) (*corev1.Node, error) | ||||||||||||||
| Patch(node *corev1.Node, patchType types.PatchType, data []byte) error | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // restrictedNodeClient uses node binding token with validating policy to avoid security problems. | ||||||||||||||
| type restrictedNodeClient struct { | ||||||||||||||
| Client client.Client | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // kubeletNodeClient uses mounted kubelet config to avoid security problems. | ||||||||||||||
| type kubeletNodeClient struct { | ||||||||||||||
| Clientset *kubernetes.Clientset | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (p *restrictedNodeClient) Get(nodeName string) (*corev1.Node, error) { | ||||||||||||||
| node := &corev1.Node{} | ||||||||||||||
| key := types.NamespacedName{Name: nodeName} | ||||||||||||||
| if err := p.Client.Get(context.TODO(), key, node); err != nil { | ||||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
| return node, nil | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (p *restrictedNodeClient) Patch(node *corev1.Node, patchType types.PatchType, data []byte) error { | ||||||||||||||
| err := p.Client.Patch(context.TODO(), node, client.RawPatch(patchType, data)) | ||||||||||||||
| return err | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+53
to
+55
|
||||||||||||||
| err := p.Client.Patch(context.TODO(), node, client.RawPatch(patchType, data)) | |
| return err | |
| } | |
| return p.Client.Patch(context.TODO(), node, client.RawPatch(patchType, data)) | |
| } | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,37 +20,35 @@ import ( | |||||||||||
| "os" | ||||||||||||
|
|
||||||||||||
| "github.com/fluid-cloudnative/fluid/pkg/csi/config" | ||||||||||||
| "github.com/fluid-cloudnative/fluid/pkg/utils/compatibility" | ||||||||||||
| "github.com/fluid-cloudnative/fluid/pkg/utils/kubelet" | ||||||||||||
| "github.com/golang/glog" | ||||||||||||
| "github.com/pkg/errors" | ||||||||||||
| "k8s.io/client-go/kubernetes" | ||||||||||||
| "sigs.k8s.io/controller-runtime/pkg/manager" | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| // getNodeAuthorizedClientFromKubeletConfig retrieves a node-authorized Kubernetes client from the Kubelet configuration file. | ||||||||||||
| // This function checks if the specified Kubelet configuration file exists. If the file does not exist, it returns an empty client without an error . | ||||||||||||
| // isUseKubeletConfig checks if the specified Kubelet configuration file exists. If the file does not exist, it returns an empty client without an error . | ||||||||||||
| // If the file exists, it attempts to initialize and return a node-authorized Kubernetes client. | ||||||||||||
|
Comment on lines
+29
to
30
|
||||||||||||
| // isUseKubeletConfig checks if the specified Kubelet configuration file exists. If the file does not exist, it returns an empty client without an error . | |
| // If the file exists, it attempts to initialize and return a node-authorized Kubernetes client. | |
| // isUseKubeletConfig checks if the specified Kubelet configuration file exists and returns true if it does, false otherwise. |
Copilot
AI
Nov 6, 2025
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.
When os.Stat fails with an error other than IsNotExist, the function logs a warning but continues execution and returns true on line 41. This means if there's a permission error or any other filesystem issue, the code will incorrectly assume the kubelet config should be used, leading to a failure when trying to initialize the client later. The function should either return false or include the error in the warning message with proper handling context.
| glog.Warningf("fail to stat kubelet config file %s", kubeletConfigPath) | |
| } | |
| glog.Warningf("fail to stat kubelet config file %s: %v, continue without node authorization...", kubeletConfigPath, err) | |
| return false | |
| } |
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.
The isUseKubeletConfig function currently returns true if os.Stat fails for reasons other than os.IsNotExist. This can lead to getNodeAuthClient attempting to initialize a kubeletNodeClient with an inaccessible config file, resulting in a subsequent error. It's safer to treat any os.Stat error (other than os.IsNotExist) as an indication that the kubelet config is not usable, and thus return false.
This ensures that if the kubelet config file exists but has permission issues, the system will correctly fall back to the restrictedNodeClient if NodeBindingToken is supported, rather than crashing trying to use an inaccessible file.
glog.Warningf("fail to stat kubelet config file %s: %v", kubeletConfigPath, err)
return false
}
return true| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,68 @@ | ||||||
| /* | ||||||
| Copyright 2025 The Fluid Authors. | ||||||
| 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 compatibility | ||||||
|
|
||||||
| import ( | ||||||
| "github.com/blang/semver/v4" | ||||||
|
|
||||||
| nativeLog "log" | ||||||
| "sync" | ||||||
|
|
||||||
| "k8s.io/apimachinery/pkg/api/errors" | ||||||
| "k8s.io/client-go/discovery" | ||||||
| ctrl "sigs.k8s.io/controller-runtime" | ||||||
| ) | ||||||
|
|
||||||
| var ( | ||||||
| nodeBindingTokenSupported = false | ||||||
| nodeBindingTokenOnce sync.Once | ||||||
| ) | ||||||
|
|
||||||
| // Beta release, default enabled. see https://github.com/kubernetes/enhancements/issues/4193 | ||||||
| const nodeBindingTokenSupportedVersion = "v1.30.0" | ||||||
|
|
||||||
| // Checks the ServiceAccountTokenPodNodeInfo feature gate, whether the apiserver embeds the node name for the associated node when issuing service account tokens bound to Pod objects. | ||||||
|
||||||
| func discoverNodeBindingTokenCompatibility() { | ||||||
| nativeLog.Printf("Discovering k8s version to check NodeBindingToken compatibility...") | ||||||
| restConfig := ctrl.GetConfigOrDie() | ||||||
| discoveryClient := discovery.NewDiscoveryClientForConfigOrDie(restConfig) | ||||||
|
|
||||||
| serverVersion, err := discoveryClient.ServerVersion() | ||||||
| if err != nil && !errors.IsNotFound(err) { | ||||||
|
||||||
| if err != nil && !errors.IsNotFound(err) { | |
| if err != nil { |
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.
Using nativeLog.Fatalf in discoverNodeBindingTokenCompatibility can cause the CSI driver to crash on startup if there's an issue discovering or parsing the Kubernetes version. This is too aggressive for a compatibility check, especially when a fallback mechanism (kubelet config) might be available.
It's better to log the error and set nodeBindingTokenSupported to false to allow the program to continue with the alternative client, improving the robustness of the CSI driver.
func discoverNodeBindingTokenCompatibility() {
nativeLog.Printf("Discovering k8s version to check NodeBindingToken compatibility...")
restConfig := ctrl.GetConfigOrDie()
discoveryClient := discovery.NewDiscoveryClientForConfigOrDie(restConfig)
serverVersion, err := discoveryClient.ServerVersion()
if err != nil {
if !errors.IsNotFound(err) {
nativeLog.Printf("ERROR: failed to discover Kubernetes server version: %v. NodeBindingToken will not be supported.", err)
} else {
nativeLog.Printf("WARNING: Kubernetes server version not found. NodeBindingToken will not be supported.")
}
nodeBindingTokenSupported = false
return
}
currentVersion, err := semver.ParseTolerant(serverVersion.GitVersion)
if err != nil {
nativeLog.Printf("ERROR: Failed to parse current Kubernetes version '%s': %v. NodeBindingToken will not be supported.", serverVersion.GitVersion, err)
nodeBindingTokenSupported = false
return
}
targetVersion, err := semver.ParseTolerant(nodeBindingTokenSupportedVersion)
if err != nil {
nativeLog.Printf("ERROR: Failed to parse target version '%s': %v. NodeBindingToken will not be supported.", nodeBindingTokenSupportedVersion, err)
nodeBindingTokenSupported = false
return
}
if currentVersion.GTE(targetVersion) {
nodeBindingTokenSupported = true
nativeLog.Printf("NodeBindingToken is supported (current version %s >= target version %s)", currentVersion, targetVersion)
} else {
nodeBindingTokenSupported = false
nativeLog.Printf("NodeBindingToken is not supported (current version %s < target version %s)", currentVersion, targetVersion)
}
}
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.
Line 65 has grammatical issues and unclear meaning. 'can only be set false' should be 'Can only be set to false'. Also, 'useless' is informal; consider 'not required' or 'ignored'. Suggested revision: 'Can only be set to false when k8s.version >= 1.30, and the kubelet.kubeConfigFile setting below will be ignored.'