Skip to content
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

Adding Credential Provider Package with source and helm chart #829

Merged
merged 16 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/gosec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ jobs:
- name: Run Gosec Security Scanner
uses: securego/gosec@master
with:
args: --exclude-dir=kubetest-plugins --exclude-dir generatebundlefile --exclude-dir ecrtokenrefresher ./...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the github workflows

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but why is gosec ignoring parts of our repo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue #870 to follow up on this

args: --exclude-dir=kubetest-plugins --exclude-dir generatebundlefile --exclude-dir ecrtokenrefresher --exclude-dir credentialproviderpackage ./...
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ vet: ## Run go vet against code.

gosec: ## Run gosec against code.
$(GO) install github.com/securego/gosec/v2/cmd/gosec@latest
gosec --exclude-dir generatebundlefile --exclude-dir ecrtokenrefresher ./...
gosec --exclude-dir generatebundlefile --exclude-dir ecrtokenrefresher --exclude-dir credentialproviderpackage ./...

SIGNED_ARTIFACTS = pkg/signature/testdata/packagebundle_minControllerVersion.yaml.signed pkg/signature/testdata/packagebundle_valid.yaml.signed pkg/signature/testdata/pod_valid.yaml.signed api/testdata/bundle_one.yaml.signed api/testdata/bundle_two.yaml.signed
ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
Expand Down
14 changes: 14 additions & 0 deletions credentialproviderpackage/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
FROM golang:1.19-buster
ENV GOTRACEBACK=single
ENV GOPROXY=direct
WORKDIR /app
COPY go.mod .
COPY go.sum .
COPY cmd/ cmd/
COPY ecr-credential-provider /eksa-binaries/
COPY aws_signing_helper /eksa-binaries/
COPY pkg/ pkg/
ARG SKAFFOLD_GO_GCFLAGS
RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o app cmd/aws-credential-provider/*.go

CMD ["/app/app"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never like that we have docker files, but don't use them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the dockerfile is useful for local testing as this binary can't really be tested locally since it requires the dependent binaries (ecr-credential-provider, iam-signing-helper) to copy.

41 changes: 41 additions & 0 deletions credentialproviderpackage/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
SHELL = /usr/bin/env bash -o pipefail
.SHELLFLAGS = -ec

REPO_ROOT=$(shell git rev-parse --show-toplevel)
PROJECT_ROOT=$(REPO_ROOT)/credentialproviderpackage
GOLANG_VERSION?="1.19"
GO ?= $(shell source $(REPO_ROOT)/scripts/common.sh && build::common::get_go_path $(GOLANG_VERSION))/go

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
ifeq (,$(shell go env GOBIN))
GOBIN=$(shell go env GOPATH)/bin
else
GOBIN=$(shell go env GOBIN)
endif

all: build

help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

clean: ## Clean output directory, and the built binary
rm -rf output/
rm -rf bin/*
rm cover.out

##@ Build

build: ## Build Binary
mkdir -p $(PROJECT_ROOT)/bin
$(GO) mod tidy -compat=$(GOLANG_VERSION)
$(GO) build -o $(PROJECT_ROOT)/bin/aws-credential-provider $(PROJECT_ROOT)/cmd/aws-credential-provider/*.go

build-linux:
[ -d bin ] || mkdir bin
env CGO_ENABLED=0 GOOS=linux $(MAKE) build

run:
$(GO) run .

test: build
$(GO) test ./... `$(GO) list $(GOTESTS) | grep -v mocks | grep -v fake | grep -v testutil` -coverprofile cover.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Patterns to ignore when building packages.
# This supports shell glob matching, relative path matching, and
# negation (prefixed with !). Only one pattern per line.
.DS_Store
# Common VCS dirs
.git/
.gitignore
.bzr/
.bzrignore
.hg/
.hgignore
.svn/
# Common backup files
*.swp
*.bak
*.tmp
*.orig
*~
# Various IDEs
.project
.idea/
*.tmproj
.vscode/
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v2
name: credential-provider-package
description: A Helm chart for credential-provider-package, an application for configuring credentials via Kubelet Credential Provider
type: application
version: 0.1.0
sources:
- https://github.com/aws/eks-anywhere-packages/credentialproviderpackage
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
{{/*
Expand the name of the chart.
*/}}
{{- define "credential-provider.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "credential-provider.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
{{- end }}

{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "credential-provider.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Common labels
*/}}
{{- define "credential-provider.labels" -}}
helm.sh/chart: {{ include "credential-provider.chart" . }}
{{ include "credential-provider.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end }}

{{/*
Selector labels
*/}}
{{- define "credential-provider.selectorLabels" -}}
app.kubernetes.io/name: {{ include "credential-provider.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

{{/*
Create the name of the service account to use
*/}}
{{- define "credential-provider.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "credential-provider.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}

{{/*
Create image name
*/}}
{{- define "template.image" -}}
{{- if eq (substr 0 7 .tag) "sha256:" -}}
{{- printf "/%s@%s" .repository .tag -}}
{{- else -}}
{{- printf "/%s:%s" .repository .tag -}}
{{- end -}}
{{- end -}}

{{/*
Function to figure out os name
*/}}
{{- define "template.getOSName" -}}
{{- with first ((lookup "v1" "Node" "" "").items) -}}
{{- if contains "Bottlerocket" .status.nodeInfo.osImage -}}
{{- printf "bottlerocket" -}}
{{- else if contains "Amazon Linux" .status.nodeInfo.osImage -}}
{{- printf "amazonlinux" -}}
{{- else -}}
{{- printf "other" -}}
{{- end }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: {{ include "credential-provider.fullname" . }}
namespace: {{ .Release.Namespace | default .Values.defaultNamespace | quote }}
lewisdiamond marked this conversation as resolved.
Show resolved Hide resolved
labels:
{{- include "credential-provider.labels" . | nindent 4 }}
spec:
selector:
matchLabels:
{{- include "credential-provider.selectorLabels" . | nindent 6 }}
template:
metadata:
{{- with .Values.podAnnotations }}
annotations:
{{- toYaml . | nindent 8 }}
{{- end }}
labels:
{{- include "credential-provider.selectorLabels" . | nindent 8 }}
spec:
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ include "credential-provider.serviceAccountName" . }}
securityContext:
{{- toYaml .Values.podSecurityContext | nindent 8 }}
containers:
- name: credential-provider
image: {{ .Values.sourceRegistry }}/{{ .Values.image.repository }}@{{ .Values.image.digest }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
securityContext:
privileged: true
resources:
{{- toYaml .Values.resources | nindent 12 }}
volumeMounts:
- name: aws-creds
mountPath: /secrets/aws-creds
{{ $os := include "template.getOSName" .}}
{{- if eq $os "bottlerocket" }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should reduce the logic here. Maybe move it to the helpers.tpl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as other comment

- mountPath: /run/api.sock
name: socket
{{- else}}
- mountPath: /node-files/kubelet-extra-args
name: kubelet-extra-args
- name: package-mounts
mountPath: /eksa-packages
{{- end}}
env:
- name: OS_TYPE
value: {{ $os }}
- name: AWS_PROFILE
value: {{.Values.application.profile}}
- name: MATCH_IMAGES
value: '{{ join "," .Values.application.matchImages }}'
- name: DEFAULT_CACHE_DURATION
value: {{.Values.application.defaultCacheDuration}}
volumes:
- name: aws-creds
secret:
secretName: {{.Values.application.secretName}}
optional: false
{{- if eq $os "bottlerocket" }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should reduce the logic here. Maybe move it to the helpers.tpl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strongly prefer to keep this logic here. Makes the daemonset on its own much easier to read then having to figure out whats mounted from helpers.tpl

- name: socket
hostPath:
path: /run/api.sock
{{- else if eq $os "amazonlinux"}}
- name: kubelet-extra-args
hostPath:
path: /etc/default/kubelet
type: FileOrCreate
{{- else}}
- name: kubelet-extra-args
hostPath:
path: /etc/sysconfig/kubelet
type: FileOrCreate
{{- end }}
{{- if ne $os "bottlerocket" }}
- name: package-mounts
hostPath:
path: /eksa-packages
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.affinity }}
affinity:
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
hostPID: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{{- if .Values.serviceAccount.create -}}
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ include "credential-provider.serviceAccountName" . }}
namespace: {{ .Release.Namespace | default .Values.defaultNamespace | quote }}
lewisdiamond marked this conversation as resolved.
Show resolved Hide resolved
labels:
{{- include "credential-provider.labels" . | nindent 4 }}
{{- with .Values.serviceAccount.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Default values for credential-provider.
# This is a YAML-formatted file.

replicaCount: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is most likely unused.

# -- sourceRegistry for all container images in chart.
sourceRegistry: public.ecr.aws/eks-anywhere
defaultNamespace: eksa-packages

image:
repository: "credential-provider-package"
tag: "{{credential-provider-package-tag}}"
digest: "{{credential-provider-package-digest}}"
pullPolicy: IfNotPresent

# application values
application:
secretName: aws-cred
matchImages: []
defaultCacheDuration: ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the chart provide a default duration?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can include one, currently the application provides one if not included by chart

profile: ""

imagePullSecrets: []
nameOverride: ""
fullnameOverride: ""

serviceAccount:
# Specifies whether a service account should be created
create: true
# Annotations to add to the service account
annotations: {}
# The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
name: ""

podAnnotations: {}

podSecurityContext: {}

securityContext: {}

resources: {}

nodeSelector: {}

tolerations: []

affinity: {}
Loading