Skip to content

Commit

Permalink
refactor: Decouple the secret resolver from the kubernetes clientset (#…
Browse files Browse the repository at this point in the history 
…1536)

* refactor(ref::secret): decouple from kubernetes clientset

* chore(*): update Makefile
  • Loading branch information
Jian Zeng committed Dec 17, 2020
1 parent a813743 commit 46853ba
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 35 deletions.
6 changes: 3 additions & 3 deletions Makefile
View file
Expand Up @@ -122,13 +122,13 @@ $(GOLANGCI_LINT):
curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(BIN_DIR) v1.20.1

test:
@go test -p $(CPUS) $$(go list ./... | grep -v /vendor | grep -v /test) -coverprofile=coverage.out
@go test -race -p $(CPUS) $$(go list ./... | grep -v /vendor | grep -v /test) -coverprofile=coverage.out
@go tool cover -func coverage.out | tail -n 1 | awk '{ print "Total coverage: " $$3 }'

build-local:
@for target in $(TARGETS); do \
CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) \
go build -i -v -o $(OUTPUT_DIR)/$${target} -p $(CPUS) \
go build -trimpath -i -v -o $(OUTPUT_DIR)/$${target} -p $(CPUS) \
-ldflags "-s -w -X $(ROOT)/pkg/server/version.VERSION=$(VERSION) \
-X $(ROOT)/pkg/server/version.COMMIT=$(COMMIT) \
-X $(ROOT)/pkg/server/version.REPOROOT=$(ROOT)" \
Expand All @@ -147,7 +147,7 @@ build-linux:
-e SHELLOPTS=$(SHELLOPTS) \
$(BASE_REGISTRY)/golang:1.13.9-stretch \
/bin/bash -c 'for target in $(TARGETS); do \
go build -i -v -o $(OUTPUT_DIR)/$${target} -p $(CPUS) \
go build -trimpath -i -v -o $(OUTPUT_DIR)/$${target} -p $(CPUS) \
-ldflags "-s -w -X $(ROOT)/pkg/server/version.VERSION=$(VERSION) \
-X $(ROOT)/pkg/server/version.COMMIT=$(COMMIT) \
-X $(ROOT)/pkg/server/version.REPOROOT=$(ROOT)" \
Expand Down
15 changes: 11 additions & 4 deletions pkg/workflow/values/ref/ref.go
View file
@@ -1,23 +1,30 @@
package ref

import (
corev1 "k8s.io/api/core/v1"

"github.com/caicloud/cyclone/pkg/apis/cyclone/v1alpha1"
"github.com/caicloud/cyclone/pkg/util/k8s"
)

// SecretGetter is used to get a secret from the cluster.
// The reason we use SecretGetter here is to decouple from the kubernetes clientset.
type SecretGetter = func(namespace, name string) (*corev1.Secret, error)

// Processor processes ref value to a string
type Processor struct {
wfr *v1alpha1.WorkflowRun
secretRefValue *SecretRefValue
variableRefValue *VariableRefValue
secretGetter SecretGetter
}

// NewProcessor creates a processor object
func NewProcessor(wfr *v1alpha1.WorkflowRun) *Processor {
func NewProcessor(wfr *v1alpha1.WorkflowRun, getter SecretGetter) *Processor {
return &Processor{
wfr: wfr,
secretRefValue: NewSecretRefValue(),
variableRefValue: NewVariableRefValue(wfr),
secretGetter: getter,
}
}

Expand All @@ -26,12 +33,12 @@ func NewProcessor(wfr *v1alpha1.WorkflowRun) *Processor {
// - '${secrets.<ns>:<secret>/<jsonpath>/...}' to refer value in a secret
// - '${stages.<stage>.outputs.<key>}' to refer value from a stage output
// - '${variables.<key>}' to refer value from a global variable defined in wfr
func (p *Processor) ResolveRefStringValue(ref string, client k8s.Interface) (string, error) {
func (p *Processor) ResolveRefStringValue(ref string) (string, error) {
var value string
var err error

if err = p.secretRefValue.Parse(ref); err == nil {
value, err = p.secretRefValue.Resolve(client)
value, err = p.secretRefValue.Resolve(p.secretGetter)
if err != nil {
return ref, err
}
Expand Down
25 changes: 15 additions & 10 deletions pkg/workflow/values/ref/ref_test.go
View file
@@ -1,6 +1,7 @@
package ref

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -58,46 +59,50 @@ func (suite *RefSuite) SetupSuite() {

func (suite *RefSuite) TestResolveRefStringValue() {
assert := assert.New(suite.T())
processor := NewProcessor(suite.wfr)

v, err := processor.ResolveRefStringValue("", suite.client)
secretGetter := func(ns, name string) (*corev1.Secret, error) {
return suite.client.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{})
}
processor := NewProcessor(suite.wfr, secretGetter)

v, err := processor.ResolveRefStringValue("")
assert.Nil(err)
assert.Equal("", v)

v, err = processor.ResolveRefStringValue("test1", suite.client)
v, err = processor.ResolveRefStringValue("test1")
assert.Nil(err)
assert.Equal("test1", v)

v, err = processor.ResolveRefStringValue("${variables.Registry}", suite.client)
v, err = processor.ResolveRefStringValue("${variables.Registry}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("docker.io", v)

v, err = processor.ResolveRefStringValue("${variables.IMAGE}", suite.client)
v, err = processor.ResolveRefStringValue("${variables.IMAGE}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("cyclone", v)

v, err = processor.ResolveRefStringValue("${variables.Registry}", suite.client)
v, err = processor.ResolveRefStringValue("${variables.Registry}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("docker.io", v)

v, err = processor.ResolveRefStringValue("${secrets.ns:secret/a.b}", suite.client)
v, err = processor.ResolveRefStringValue("${secrets.ns:secret/a.b}")
assert.NotNil(v)
assert.Error(err)

v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.key}", suite.client)
v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.key}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("key1", v)

v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.json/user.languages[1]}", suite.client)
v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.json/user.languages[1]}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("go", v)

v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.json/user.name}", suite.client)
v, err = processor.ResolveRefStringValue("${secrets.ns:secret/data.json/user.name}")
assert.NotNil(v)
assert.Nil(err)
assert.Equal("cyclone", v)
Expand Down
8 changes: 2 additions & 6 deletions pkg/workflow/values/ref/secret.go
View file
@@ -1,7 +1,6 @@
package ref

import (
"context"
"encoding/base64"
"encoding/json"
"errors"
Expand All @@ -10,9 +9,6 @@ import (
"strings"

"github.com/PaesslerAG/jsonpath"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/caicloud/cyclone/pkg/util/k8s"
)

var (
Expand Down Expand Up @@ -127,7 +123,7 @@ func (r *SecretRefValue) parseOld(ref string) error {
}

// Resolve resolves the secret ref and get the real value.
func (r *SecretRefValue) Resolve(client k8s.Interface) (string, error) {
func (r *SecretRefValue) Resolve(secretGetter SecretGetter) (string, error) {
if len(r.Secret) == 0 || len(r.Jsonpaths) == 0 {
return "", errors.New("empty secret name or jsonpath")
}
Expand All @@ -136,7 +132,7 @@ func (r *SecretRefValue) Resolve(client k8s.Interface) (string, error) {
r.Namespace = "default"
}

s, err := client.CoreV1().Secrets(r.Namespace).Get(context.TODO(), r.Secret, metav1.GetOptions{})
s, err := secretGetter(r.Namespace, r.Secret)
if err != nil {
return "", fmt.Errorf("get secret %s:%s error: %v", r.Namespace, r.Secret, err)
}
Expand Down
23 changes: 15 additions & 8 deletions pkg/workflow/values/ref/secret_test.go
View file
@@ -1,6 +1,7 @@
package ref

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -65,24 +66,27 @@ func (suite *SecretSuite) TestResolveOld() {
assert := assert.New(suite.T())
secretRefValue := NewSecretRefValue()
assert.Nil(secretRefValue.Parse("$.ns.secret/a.b"))
v, err := secretRefValue.Resolve(suite.client)
secretGetter := func(ns, name string) (*corev1.Secret, error) {
return suite.client.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{})
}
v, err := secretRefValue.Resolve(secretGetter)
assert.Empty(v)
assert.Error(err)

assert.Nil(secretRefValue.Parse("$.ns.secret/data.key"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("key1", v)

assert.Nil(secretRefValue.Parse("$.ns.secret/data.json/user.languages[1]"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("go", v)

assert.Nil(secretRefValue.Parse("$.ns.secret/data.json/user.name"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("cyclone", v)
Expand All @@ -92,24 +96,27 @@ func (suite *SecretSuite) TestResolve() {
assert := assert.New(suite.T())
secretRefValue := NewSecretRefValue()
assert.Nil(secretRefValue.Parse("${secrets.ns:secret/a.b}"))
v, err := secretRefValue.Resolve(suite.client)
secretGetter := func(ns, name string) (*corev1.Secret, error) {
return suite.client.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{})
}
v, err := secretRefValue.Resolve(secretGetter)
assert.Empty(v)
assert.Error(err)

assert.Nil(secretRefValue.Parse("${secrets.ns:secret/data.key}"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("key1", v)

assert.Nil(secretRefValue.Parse("${secrets.ns:secret/data.json/user.languages[1]}"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("go", v)

assert.Nil(secretRefValue.Parse("${secrets.ns:secret/data.json/user.name}"))
v, err = secretRefValue.Resolve(suite.client)
v, err = secretRefValue.Resolve(secretGetter)
assert.NotNil(v)
assert.Nil(err)
assert.Equal("cyclone", v)
Expand Down
11 changes: 7 additions & 4 deletions pkg/workflow/workload/pod/builder.go
View file
Expand Up @@ -43,6 +43,9 @@ type Builder struct {

// NewBuilder creates a new pod builder.
func NewBuilder(client k8s.Interface, wf *v1alpha1.Workflow, wfr *v1alpha1.WorkflowRun, stg *v1alpha1.Stage) *Builder {
secretGetter := func(ns, name string) (*corev1.Secret, error) {
return client.CoreV1().Secrets(ns).Get(context.TODO(), name, metav1.GetOptions{})
}
return &Builder{
client: client,
wf: wf,
Expand All @@ -52,7 +55,7 @@ func NewBuilder(client k8s.Interface, wf *v1alpha1.Workflow, wfr *v1alpha1.Workf
pod: &corev1.Pod{},
pvcVolumes: make(map[string]string),
executionContext: GetExecutionContext(wfr),
refProcessor: ref.NewProcessor(wfr),
refProcessor: ref.NewProcessor(wfr, secretGetter),
}
}

Expand Down Expand Up @@ -141,7 +144,7 @@ func (m *Builder) ResolveArguments() error {

for k, v := range parameters {
v = values.GenerateValue(v)
resolved, err := m.refProcessor.ResolveRefStringValue(v, m.client)
resolved, err := m.refProcessor.ResolveRefStringValue(v)
if err != nil {
log.WithField("key", k).WithField("value", v).Error("resolve ref failed:", err)
return err
Expand Down Expand Up @@ -423,7 +426,7 @@ func (m *Builder) ResolveInputResources() error {
}
var envs []corev1.EnvVar
for key, value := range envsMap {
resolved, err := m.refProcessor.ResolveRefStringValue(value, m.client)
resolved, err := m.refProcessor.ResolveRefStringValue(value)
if err != nil {
return fmt.Errorf("resolve ref value '%s' error: %v", value, err)
}
Expand Down Expand Up @@ -536,7 +539,7 @@ func (m *Builder) ResolveOutputResources() error {
}
var envs []corev1.EnvVar
for key, value := range envsMap {
resolved, err := m.refProcessor.ResolveRefStringValue(value, m.client)
resolved, err := m.refProcessor.ResolveRefStringValue(value)
if err != nil {
return fmt.Errorf("resolve ref value '%s' error: %v", value, err)
}
Expand Down

0 comments on commit 46853ba

Please sign in to comment.