-
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 | |
| } |
| 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.
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.'