Skip to content

Commit

Permalink
Merge pull request #681 from jonathanrainer/fix-imds-dependency
Browse files Browse the repository at this point in the history
Removing Dependency on IMDS, allowing `hostNetwork: true` to be removed
  • Loading branch information
k8s-ci-robot authored Jun 15, 2022
2 parents f2597bb + a17603b commit ac6ade7
Show file tree
Hide file tree
Showing 227 changed files with 22,862 additions and 167 deletions.
15 changes: 15 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,21 @@ test-e2e:
GINKGO_FOCUS="\[efs-csi\]" \
./hack/e2e/run.sh

.PHONY: test-e2e-external-eks
test-e2e-external-eks:
CLUSTER_TYPE=eksctl \
K8S_VERSION="1.20" \
DRIVER_NAME=aws-efs-csi-driver \
HELM_VALUES_FILE="./hack/values_eksctl.yaml" \
CONTAINER_NAME=efs-plugin \
TEST_EXTRA_FLAGS='--cluster-name=$$CLUSTER_NAME' \
AWS_REGION=us-west-2 \
AWS_AVAILABILITY_ZONES=us-west-2a,us-west-2b,us-west-2c \
TEST_PATH=./test/e2e/... \
GINKGO_FOCUS="\[efs-csi\]" \
EKSCTL_ADMIN_ROLE="Infra-prod-KopsDeleteAllLambdaServiceRoleF1578477-1ELDFIB4KCMXV" \
./hack/e2e/run.sh

.PHONY: test-e2e-bin
test-e2e-bin:
mkdir -p bin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ spec:
annotations: {{- toYaml . | nindent 8 }}
{{- end }}
spec:
hostNetwork: true
{{- if .Values.imagePullSecrets }}
imagePullSecrets:
{{- range .Values.imagePullSecrets }}
Expand Down Expand Up @@ -62,6 +61,10 @@ spec:
- name: AWS_STS_REGIONAL_ENDPOINTS
value: regional
{{- end }}
- name: CSI_NODE_NAME
valueFrom:
fieldRef:
fieldPath: spec.nodeName
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
Expand Down
5 changes: 4 additions & 1 deletion deploy/kubernetes/base/controller-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ spec:
app.kubernetes.io/name: aws-efs-csi-driver
app.kubernetes.io/instance: kustomize
spec:
hostNetwork: true
nodeSelector:
kubernetes.io/os: linux
serviceAccountName: efs-csi-controller-sa
Expand All @@ -41,6 +40,10 @@ spec:
env:
- name: CSI_ENDPOINT
value: unix:///var/lib/csi/sockets/pluginproxy/csi.sock
- name: CSI_NODE_NAME
valueFrom:
fieldRef:
fieldPath: spec.nodeName
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
Expand Down
3 changes: 0 additions & 3 deletions hack/e2e/eksctl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ function eksctl_create_cluster() {
fi

loudecho "Cluster ${CLUSTER_NAME} kubecfg written to ${KUBECONFIG}"
# TODO: Workaround for https://github.com/weaveworks/eksctl/issues/5257
# Remove when eksctl releases a fix
sed -i 's/v1alpha1/v1beta1/g' ${KUBECONFIG}

loudecho "Getting cluster ${CLUSTER_NAME}"
${BIN} get cluster "${CLUSTER_NAME}"
Expand Down
4 changes: 2 additions & 2 deletions hack/e2e/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ KOPS_STATE_FILE=${KOPS_STATE_FILE:-s3://k8s-kops-csi-e2e}
KOPS_PATCH_FILE=${KOPS_PATCH_FILE:-./hack/kops-patch.yaml}
KOPS_PATCH_NODE_FILE=${KOPS_PATCH_NODE_FILE:-./hack/kops-patch-node.yaml}

EKSCTL_VERSION=${EKSCTL_VERSION:-0.69.0}
EKSCTL_VERSION=${EKSCTL_VERSION:-0.100.0}
EKSCTL_PATCH_FILE=${EKSCTL_PATCH_FILE:-./hack/eksctl-patch.yaml}
EKSCTL_ADMIN_ROLE=${EKSCTL_ADMIN_ROLE:-}
# Creates a windows node group. The windows ami doesn't (yet) install csi-proxy
Expand All @@ -69,7 +69,7 @@ HELM_EXTRA_FLAGS=${HELM_EXTRA_FLAGS:-}

TEST_PATH=${TEST_PATH:-"./tests/e2e/..."}
ARTIFACTS=${ARTIFACTS:-"${TEST_DIR}/artifacts"}
GINKGO_FOCUS=${GINKGO_FOCUS:-"\[ebs-csi-e2e\]"}
GINKGO_FOCUS=${GINKGO_FOCUS:-"\[efs-csi]"}
GINKGO_SKIP=${GINKGO_SKIP:-"\[Disruptive\]"}
GINKGO_NODES=${GINKGO_NODES:-4}
TEST_EXTRA_FLAGS=${TEST_EXTRA_FLAGS:-}
Expand Down
58 changes: 40 additions & 18 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@ import (
"context"
"errors"
"fmt"
"math/rand"
"os"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/credentials/stscreds"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/efs"
"k8s.io/klog"
"math/rand"
"time"
)

const (
Expand Down Expand Up @@ -100,36 +103,50 @@ type cloud struct {
// NewCloud returns a new instance of AWS cloud
// It panics if session is invalid
func NewCloud() (Cloud, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
metadata, err := NewMetadataService(sess)
if err != nil {
return nil, fmt.Errorf("could not get metadata from AWS: %v", err)
}

efsClient := efs.New(session.Must(session.NewSession(aws.NewConfig().WithRegion(metadata.GetRegion()))))
return &cloud{
metadata: metadata,
efs: efsClient,
}, nil
return createCloud("")
}

// NewCloudWithRole returns a new instance of AWS cloud after assuming an aws role
// It panics if driver does not have permissions to assume role.
func NewCloudWithRole(awsRoleArn string) (Cloud, error) {
return createCloud(awsRoleArn)
}

func createCloud(awsRoleArn string) (Cloud, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
metadata, err := NewMetadataService(sess)
svc := ec2metadata.New(sess)
api, err := DefaultKubernetesAPIClient()

if err != nil && !isDriverBootedInECS() {
klog.Warningf("Could not create Kubernetes Client: %v", err)
}

metadataProvider, err := GetNewMetadataProvider(svc, api)

if err != nil {
return nil, fmt.Errorf("Could not get metadata from AWS: %v", err)
return nil, fmt.Errorf("error creating MetadataProvider: %v", err)
}

metadata, err := metadataProvider.getMetadata()

if err != nil {
return nil, fmt.Errorf("could not get metadata: %v", err)
}

creds := stscreds.NewCredentials(sess, awsRoleArn)
efsClient := efs.New(session.Must(session.NewSession(aws.NewConfig().WithCredentials(creds).WithRegion(metadata.GetRegion()))))
return &cloud{
metadata: metadata,
efs: efsClient,
efs: createEfsClient(awsRoleArn, metadata, sess),
}, nil
}

func createEfsClient(awsRoleArn string, metadata MetadataService, sess *session.Session) Efs {
config := aws.NewConfig().WithRegion(metadata.GetRegion())
if awsRoleArn != "" {
config = config.WithCredentials(stscreds.NewCredentials(sess, awsRoleArn))
}
return efs.New(session.Must(session.NewSession(config)))
}

func (c *cloud) GetMetadata() MetadataService {
return c.metadata
}
Expand Down Expand Up @@ -309,6 +326,11 @@ func isAccessDenied(err error) bool {
return false
}

func isDriverBootedInECS() bool {
ecsContainerMetadataUri := os.Getenv(taskMetadataV4EnvName)
return ecsContainerMetadataUri != ""
}

func parseEfsTags(tagMap map[string]string) []*efs.Tag {
efsTags := []*efs.Tag{}
for k, v := range tagMap {
Expand Down
41 changes: 41 additions & 0 deletions pkg/cloud/ec2_metadata.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package cloud

import (
"fmt"

"github.com/aws/aws-sdk-go/aws/ec2metadata"
)

type EC2Metadata interface {
Available() bool
GetInstanceIdentityDocument() (ec2metadata.EC2InstanceIdentityDocument, error)
}

type ec2MetadataProvider struct {
ec2MetadataService EC2Metadata
}

func (e ec2MetadataProvider) getMetadata() (MetadataService, error) {
doc, err := e.ec2MetadataService.GetInstanceIdentityDocument()
if err != nil {
return nil, fmt.Errorf("could not get EC2 instance identity metadata")
}

if len(doc.InstanceID) == 0 {
return nil, fmt.Errorf("could not get valid EC2 instance ID")
}

if len(doc.Region) == 0 {
return nil, fmt.Errorf("could not get valid EC2 region")
}

if len(doc.AvailabilityZone) == 0 {
return nil, fmt.Errorf("could not get valid EC2 availavility zone")
}

return &metadata{
instanceID: doc.InstanceID,
region: doc.Region,
availabilityZone: doc.AvailabilityZone,
}, nil
}
119 changes: 119 additions & 0 deletions pkg/cloud/ec2_metadata_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package cloud

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/golang/mock/gomock"

"github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud/mocks"
)

var (
stdInstanceID = "instance-1"
stdRegionName = "instance-1"
stdAvailabilityZone = "az-1"
)

func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) {
testCases := []struct {
name string
isAvailable bool
isPartial bool
identityDocument ec2metadata.EC2InstanceIdentityDocument
err error
}{
{
name: "success: normal",
isAvailable: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
Region: stdRegionName,
AvailabilityZone: stdAvailabilityZone,
},
err: nil,
},
{
name: "fail: GetInstanceIdentityDocument returned error",
isAvailable: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
Region: stdRegionName,
AvailabilityZone: stdAvailabilityZone,
},
err: fmt.Errorf(""),
},
{
name: "fail: GetInstanceIdentityDocument returned empty instance",
isAvailable: true,
isPartial: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: "",
Region: stdRegionName,
AvailabilityZone: stdAvailabilityZone,
},
err: nil,
},
{
name: "fail: GetInstanceIdentityDocument returned empty region",
isAvailable: true,
isPartial: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
Region: "",
AvailabilityZone: stdAvailabilityZone,
},
err: nil,
},
{
name: "fail: GetInstanceIdentityDocument returned empty az",
isAvailable: true,
isPartial: true,
identityDocument: ec2metadata.EC2InstanceIdentityDocument{
InstanceID: stdInstanceID,
Region: stdRegionName,
AvailabilityZone: "",
},
err: nil,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mockCtrl := gomock.NewController(t)
mockEC2Metadata := mocks.NewMockEC2Metadata(mockCtrl)

if tc.isAvailable {
mockEC2Metadata.EXPECT().GetInstanceIdentityDocument().Return(tc.identityDocument, tc.err)
}

ec2Mp := ec2MetadataProvider{ec2MetadataService: mockEC2Metadata}
m, err := ec2Mp.getMetadata()

if tc.isAvailable && tc.err == nil && !tc.isPartial {
if err != nil {
t.Fatalf("getEC2Metadata() failed: expected no error, got %v", err)
}

if m.GetInstanceID() != tc.identityDocument.InstanceID {
t.Fatalf("GetInstanceID() failed: expected %v, got %v", tc.identityDocument.InstanceID, m.GetInstanceID())
}

if m.GetRegion() != tc.identityDocument.Region {
t.Fatalf("GetRegion() failed: expected %v, got %v", tc.identityDocument.Region, m.GetRegion())
}

if m.GetAvailabilityZone() != tc.identityDocument.AvailabilityZone {
t.Fatalf("GetAvailabilityZone() failed: expected %v, got %v", tc.identityDocument.AvailabilityZone, m.GetAvailabilityZone())
}
} else {
if err == nil {
t.Fatal("getEC2Metadata() failed: expected error when GetInstanceIdentityDocument returns partial data, got nothing")
}
}

mockCtrl.Finish()
})
}
}
Loading

0 comments on commit ac6ade7

Please sign in to comment.