From 8253d6026e12d1442795079787ac24db00c3a7d2 Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Thu, 23 Sep 2021 00:46:40 -0700 Subject: [PATCH 1/7] Parse Secrets Groups --- go.mod | 6 +- go.sum | 24 ++ pkg/log/messages/error_messages.go | 7 + pkg/secrets/push_to_file/secrets_groups.go | 235 ++++++++++++++++++ .../push_to_file/secrets_groups_test.go | 178 +++++++++++++ 5 files changed, 448 insertions(+), 2 deletions(-) create mode 100644 pkg/secrets/push_to_file/secrets_groups.go create mode 100644 pkg/secrets/push_to_file/secrets_groups_test.go diff --git a/go.mod b/go.mod index d6ad514a..dc18ba85 100644 --- a/go.mod +++ b/go.mod @@ -4,15 +4,16 @@ go 1.15 require ( github.com/cenkalti/backoff v2.2.1+incompatible - github.com/cyberark/conjur-api-go v0.6.0 + github.com/cyberark/conjur-api-go v0.8.0 github.com/cyberark/conjur-authn-k8s-client v0.19.1 github.com/gogo/protobuf v1.3.2 // indirect github.com/googleapis/gnostic v0.3.1 // indirect github.com/json-iterator/go v1.1.9 // indirect github.com/modern-go/reflect2 v1.0.1 // indirect github.com/onsi/ginkgo v1.14.0 // indirect - github.com/smartystreets/goconvey v0.0.0-20190731233626-505e41936337 + github.com/smartystreets/goconvey v1.6.4 github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/testify v1.7.0 golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f // indirect golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 // indirect golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect @@ -20,6 +21,7 @@ require ( gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.3.0 + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b k8s.io/api v0.0.0-20190313235455-40a48860b5ab k8s.io/apimachinery v0.0.0-20190313205120-d7deff9243b1 k8s.io/client-go v11.0.0+incompatible diff --git a/go.sum b/go.sum index 3c14a1be..bf3292e6 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,13 @@ cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= +github.com/axw/gocov v1.0.0/go.mod h1:LvQpEYiwwIb2nYkXY2fDWhg9/AsYqkhmrCshjlUJECE= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4= github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/cyberark/conjur-api-go v0.6.0 h1:QQYmFRhcCvmtZ9oSRoXCxWb7uRjppfu5lcEwo4HEjtg= github.com/cyberark/conjur-api-go v0.6.0/go.mod h1:uM96pLpckwYYAWRSbrsw+TT0y3kg49QCEGpdpa9dJ34= +github.com/cyberark/conjur-api-go v0.8.0 h1:y7/l7tv92h8rmpnOkebYwz/KSqUWe+IrLVITBk3ZWVQ= +github.com/cyberark/conjur-api-go v0.8.0/go.mod h1:HZ5RoBhAB2KwnxyXbQ29DwpviRVg7SMRq7QhwtFjN3Q= github.com/cyberark/conjur-authn-k8s-client v0.19.0 h1:zHjdKyZ8bu4cRyV3iYMh1/XfIVtTugoiU/CflnboP9Q= github.com/cyberark/conjur-authn-k8s-client v0.19.0/go.mod h1:qacUJXCppU1Rg/C+br9B1jBitTq4yG04oc4a+cfI200= github.com/cyberark/conjur-authn-k8s-client v0.19.1 h1:/o7De4Br4p1j2p9gOPQuurkdjypiHlmg+k2GwoGd1ik= @@ -42,6 +45,8 @@ github.com/googleapis/gnostic v0.3.1/go.mod h1:on+2t9HRStVgn95RSsFWFz+6Q0Snyqv1a github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e h1:JKmoR8x90Iww1ks85zJ1lfDGgIiMDuIptTOhJq+zKyg= github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= +github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 h1:l5lAOZEym3oK3SQ2HBHWsJUfbNBiTXJDeW2QDxw9AQ0= +github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/json-iterator/go v1.1.9 h1:9yzud/Ht36ygwatGx56VwCZtlI/2AD15T1X2sjSuGns= github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= @@ -59,10 +64,12 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0HfGg= github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= github.com/onsi/ginkgo v1.14.0 h1:2mOpI4JVVPBN+WQRa0WKH2eXR+Ey+uK4n7Zj0aYpIQA= github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= +github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA= github.com/onsi/gomega v1.10.1 h1:o0+MgICZLuZ7xjH7Vx6zS/zcu93/BEp1VwkIW1mEXCE= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -72,15 +79,22 @@ github.com/sirupsen/logrus v1.0.5/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjM github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/assertions v0.0.0-20190215210624-980c5ac6f3ac h1:wbW+Bybf9pXxnCFAOWZTqkRjAc7rAIwo2e1ArUhiHxg= github.com/smartystreets/assertions v0.0.0-20190215210624-980c5ac6f3ac/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= +github.com/smartystreets/assertions v1.2.0 h1:42S6lae5dvLc7BrLu/0ugRtcFVjoJNMC/N3yZFZkDFs= +github.com/smartystreets/assertions v1.2.0/go.mod h1:tcbTF8ujkAEcZ8TElKY+i30BzYlVhC/LOxJk7iOWnoo= github.com/smartystreets/goconvey v0.0.0-20190222223459-a17d461953aa/go.mod h1:2RVY1rIf+2J2o/IM9+vPq9RzmHDSseB7FoXiSNIUsoU= github.com/smartystreets/goconvey v0.0.0-20190731233626-505e41936337 h1:WN9BUFbdyOsSH/XohnWpXOlq9NBD5sGAB2FciQMUEe8= github.com/smartystreets/goconvey v0.0.0-20190731233626-505e41936337/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= +github.com/smartystreets/goconvey v1.6.4 h1:fv0U8FUIMPNf1L9lnHLvLhgicrIVChEkdzIKYqbNC9s= +github.com/smartystreets/goconvey v1.6.4/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20180621125126-a49355c7e3f8/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= @@ -93,6 +107,7 @@ golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f h1:aZp0e2vLN4MToVqnjNEYEt golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -126,6 +141,9 @@ golang.org/x/sys v0.0.0-20200519105757-fe76b779f299 h1:DYfZAGf2WMFjMxbgTjaC+2HC7 golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f h1:+Nyd8tzPX9R7BWHguqsrbFdRx3WQ/1ib8I44HXV5yTA= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -137,9 +155,11 @@ golang.org/x/time v0.0.0-20191024005414-555d28b269f0 h1:/5xXl8Y5W96D+TtHSlonuFqG golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= @@ -172,6 +192,10 @@ gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.0.0-20190313235455-40a48860b5ab h1:DG9A67baNpoeweOy2spF1OWHhnVY5KR7/Ek/+U1lVZc= k8s.io/api v0.0.0-20190313235455-40a48860b5ab/go.mod h1:iuAfoD4hCxJ8Onx9kaTIt30j7jUFS00AXQi6QMi99vA= k8s.io/apimachinery v0.0.0-20190313205120-d7deff9243b1 h1:IS7K02iBkQXpCeieSiyJjGoLSdVOv2DbPaWHJ+ZtgKg= diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go index e7d9f4ca..8d6a4d7e 100644 --- a/pkg/log/messages/error_messages.go +++ b/pkg/log/messages/error_messages.go @@ -56,3 +56,10 @@ const CSPFK037E string = "CSPFK037E Failed to parse DAP/Conjur variable IDs" // General const CSPFK038E string = "CSPFK038E Retransmission backoff exhausted" const CSPFK039E string = "CSPFK039E Secrets Provider for Kubernetes failed to update Kubernetes Secrets" + +// Push to File +const CSPFK051E string = "CSPFK051E Failed to unmarshal Push-to-File secrets. Reason: %s in group '%s'" +const CSPFK052E string = "CSPFK052E Annotation '%s' must contain relative path without leading '/'" +const CSPFK053E string = "CSPFK053E Annotation '%s' specifies unknown file format '%s'" +const CSPFK054E string = "CSPFK054E Failed to parse template in annotation '%s'. Reason: %s" +const CSPFK055E string = "CSPFK055E Push-to-File template specified but directory found in annotation '%s'" diff --git a/pkg/secrets/push_to_file/secrets_groups.go b/pkg/secrets/push_to_file/secrets_groups.go new file mode 100644 index 00000000..73d54f24 --- /dev/null +++ b/pkg/secrets/push_to_file/secrets_groups.go @@ -0,0 +1,235 @@ +package secrets_groups + +import ( + "fmt" + "os" + "path" + "strings" + "text/template" + + "github.com/cyberark/conjur-authn-k8s-client/pkg/log" + "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" + "gopkg.in/yaml.v3" +) + +const SECRETS_GROUP_PREFIX = "conjur.org/conjur-secrets." +const SECRETS_GROUP_POLICY_PATH_PREFIX = "conjur.org/conjur-secrets-policy-path." +const SECRETS_GROUP_FILE_PATH_PREFIX = "conjur.org/secret-file-path." +const SECRETS_GROUP_FILE_TYPE_PREFIX = "conjur.org/secret-file-format." +const SECRETS_GROUP_FILE_TEMPLATE_PREFIX = "conjur.org/secret-file-template." + +const DEFAULT_FILE_PERMS os.FileMode = 0777 +const DEFAULT_FILE_PATH string = "/conjur/secrets/" + +type SecretsFileType int + +const ( + FILE_TYPE_NONE SecretsFileType = iota + FILE_TYPE_YAML + FILE_TYPE_JSON + FILE_TYPE_DOTENV + FILE_TYPE_BASH + FILE_TYPE_PLAINTEXT +) + +// SecretsPaths comprises Conjur variable paths for all secrets in a secrets +// group, indexed by secret name. +type SecretsPaths map[string]string + +// SecretsGroupInfo comprises secrets mapping information for a given secrets +// group. +type SecretsGroupInfo struct { + Secrets SecretsPaths + FilePath string + FileType SecretsFileType + FilePerms os.FileMode + FileTemplate *template.Template +} + +// SecretsGroups comprises secrets mapping info for all secret groups +type SecretsGroups map[string]SecretsGroupInfo + +func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (SecretsGroups, error) { + secretsGroups := make(SecretsGroups) + + for annotation := range annotations { + if strings.HasPrefix(annotation, SECRETS_GROUP_PREFIX) { + groupName := strings.TrimPrefix(annotation, SECRETS_GROUP_PREFIX) + + secretsPathPrefix, err := parseSecretsPathPrefix(groupName, annotations[SECRETS_GROUP_POLICY_PATH_PREFIX+groupName]) + if err != nil { + return nil, err + } + + secrets, err := parseSecretsPaths(groupName, annotations[SECRETS_GROUP_PREFIX+groupName], secretsPathPrefix) + if err != nil { + return nil, err + } + + filePath, err := parseFilePath(groupName, annotations[SECRETS_GROUP_FILE_PATH_PREFIX+groupName]) + if err != nil { + return nil, err + } + + fileType, err := parseFileType(groupName, annotations[SECRETS_GROUP_FILE_TYPE_PREFIX+groupName]) + if err != nil { + return nil, err + } + + fileTemplate, err := parseFileTemplate(groupName, annotations[SECRETS_GROUP_FILE_TEMPLATE_PREFIX+groupName]) + if err != nil { + return nil, err + } + + if fileTemplate != nil { + fileType = FILE_TYPE_NONE + } + + groupInfo := SecretsGroupInfo{ + Secrets: secrets, + FilePath: filePath, + FileType: fileType, + FilePerms: DEFAULT_FILE_PERMS, + FileTemplate: fileTemplate, + } + + err = validateGroupInfo(groupName, groupInfo) + if err != nil { + return nil, err + } + + /* + if groupInfo.FileTemplate != nil { + groupInfo.FileTemplate.Execute(os.Stdout, groupInfo.Secrets) + } + */ + + secretsGroups[groupName] = groupInfo + } + } + + return secretsGroups, nil +} + +func parseSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix string) (SecretsPaths, error) { + secrets := make(SecretsPaths) + + unpacked := []interface{}{} + if err := yaml.Unmarshal([]byte(secretsPaths), &unpacked); err != nil { + return nil, log.RecordedError(messages.CSPFK051E, err.Error(), groupName) + } + + insertSecret := func(name string, secretPath string) error { + secretPath = path.Clean(secretPath) + if secretPath == "." { + return log.RecordedError(messages.CSPFK051E, "Invalid secret path", groupName) + } + secrets[name] = secretsPathsPrefix + secretPath + + return nil + } + + for _, secret := range unpacked { + switch val := secret.(type) { + case string: + { + name := val[strings.LastIndex(val, "/")+1:] + if name == "" { + return nil, log.RecordedError(messages.CSPFK051E, "Invalid secret name", groupName) + } + + if err := insertSecret(name, val); err != nil { + return nil, err + } + } + + case map[string]interface{}: + { + for alias, secretPath := range val { + secretPath, _ := secretPath.(string) + if err := insertSecret(alias, secretPath); err != nil { + return nil, err + } + } + } + + default: + return nil, log.RecordedError(messages.CSPFK051E, "Unknown secret format", groupName) + } + } + + if len(secrets) == 0 { + return nil, log.RecordedError(messages.CSPFK051E, "No valid secrets found", groupName) + } + + return secrets, nil +} + +func parseSecretsPathPrefix(groupName string, secretsPathPrefix string) (string, error) { + secretsPathPrefix = path.Clean("/" + secretsPathPrefix) + if secretsPathPrefix == "." || secretsPathPrefix == "/" { + return "/", nil + } + + return secretsPathPrefix + "/", nil +} + +func parseFilePath(groupName string, filePath string) (string, error) { + if strings.HasPrefix(filePath, "/") { + return "", log.RecordedError(messages.CSPFK052E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_PATH_PREFIX, groupName)) + } + + if strings.HasSuffix(filePath, "/") { + filePath = path.Clean(filePath) + "/" + } else { + filePath = path.Clean(filePath) + } + + if filePath == "." { + return DEFAULT_FILE_PATH, nil + } + + return DEFAULT_FILE_PATH + filePath, nil +} + +func parseFileType(groupName string, fileType string) (SecretsFileType, error) { + switch strings.ToLower(fileType) { + case "": + return FILE_TYPE_YAML, nil + case "yaml": + return FILE_TYPE_YAML, nil + case "json": + return FILE_TYPE_JSON, nil + case "dotenv": + return FILE_TYPE_DOTENV, nil + case "bash": + return FILE_TYPE_BASH, nil + case "plaintext": + return FILE_TYPE_PLAINTEXT, nil + default: + return FILE_TYPE_NONE, log.RecordedError(messages.CSPFK053E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_TYPE_PREFIX, groupName), fileType) + } +} + +func parseFileTemplate(groupName string, fileTemplate string) (*template.Template, error) { + if fileTemplate == "" { + return nil, nil + } + + t, err := template.New("FileTemplate").Parse(fileTemplate) + if err != nil { + return nil, log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_TEMPLATE_PREFIX, groupName), err.Error()) + } + + return t, nil +} + +func validateGroupInfo(groupName string, groupInfo SecretsGroupInfo) error { + if groupInfo.FileTemplate != nil { + if strings.HasSuffix(groupInfo.FilePath, "/") { + return log.RecordedError(messages.CSPFK055E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_PATH_PREFIX, groupName)) + } + } + + return nil +} diff --git a/pkg/secrets/push_to_file/secrets_groups_test.go b/pkg/secrets/push_to_file/secrets_groups_test.go new file mode 100644 index 00000000..657fb6f0 --- /dev/null +++ b/pkg/secrets/push_to_file/secrets_groups_test.go @@ -0,0 +1,178 @@ +package secrets_groups + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type extractSecretsGroupsTestCase struct { + description string + contents map[string]string + assert func(t *testing.T, result SecretsGroups, err error) +} + +func assertValidSecretsGroups(expected SecretsGroups) func(*testing.T, SecretsGroups, error) { + return func(t *testing.T, result SecretsGroups, err error) { + if !assert.NoError(t, err) { + return + } + + assert.Equal(t, len(expected), len(result)) + for expectedKey, expectedVal := range expected { + resultVal, ok := result[expectedKey] + assert.True(t, ok) + + assert.Equal(t, expectedVal.Secrets, resultVal.Secrets) + assert.Equal(t, expectedVal.FilePath, resultVal.FilePath) + assert.Equal(t, expectedVal.FileType, resultVal.FileType) + assert.Equal(t, expectedVal.FilePerms, resultVal.FilePerms) + + if expectedVal.FileTemplate != nil { + assert.NotNil(t, resultVal.FileTemplate) + } + } + } +} + +func assertProperError(expectedErr string) func(*testing.T, SecretsGroups, error) { + return func(t *testing.T, result SecretsGroups, err error) { + assert.Nil(t, result) + assert.Contains(t, err.Error(), expectedErr) + } +} + +const someValidSecrets = "- test/url\n- test-password: test/password\n- test-username: test/username\n" +const someValidTemplate = "\"cache\": {\n\t\"url\": {{ .url }},\n\t\"admin-password\": {{ index . \"test-password\" }},\n\t\"admin-username\": {{ index . \"test-username\" }},\n\t\"port\": 123456,\n}" + +var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ + { + description: "valid annotations map", + contents: map[string]string{ + "conjur.org/authn-identity": "host/conjur/authn-k8s/cluster/apps/inventory-api", + "conjur.org/container-mode": "application", + "conjur.org/secrets-destination": "file", + "conjur.org/k8s-secrets": "- k8s-secret-1\n- k8s-secret-2\n", + "conjur.org/retry-count-limit": "10", + "conjur.org/retry-interval-sec": "5", + "conjur.org/debug-logging": "true", + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "this/relative/path", + "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", + }, + assert: assertValidSecretsGroups( + SecretsGroups{ + "cache": { + Secrets: SecretsPaths{ + "test-password": "/some/policy/path/test/password", + "test-username": "/some/policy/path/test/username", + "url": "/some/policy/path/test/url", + }, + FilePath: "/conjur/secrets/this/relative/path", + FileType: SecretsFileType(FILE_TYPE_YAML), + FilePerms: DEFAULT_FILE_PERMS, + FileTemplate: nil, + }, + }, + ), + }, + { + description: "invalid file path", + contents: map[string]string{ + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "/this/absolute/path", + }, + assert: assertProperError("must contain relative path"), + }, + { + description: "invalid file path with template specified", + contents: map[string]string{ + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "this/relative/directory/", + "conjur.org/secret-file-template.cache": someValidTemplate, + }, + assert: assertProperError("template specified but directory found"), + }, +} + +func TestExtractSecretsGroupsFromAnnotations(t *testing.T) { + for _, tc := range extractSecretsGroupsTestCases { + t.Run(tc.description, func(t *testing.T) { + secretGroups, err := ExtractSecretsGroupsFromAnnotations(tc.contents) + + tc.assert(t, secretGroups, err) + }) + } +} + +func TestExtractFileTypeFromAnnotations(t *testing.T) { + type fileTypeTestCase struct { + input string + output SecretsFileType + valid bool + hasTemplate bool + } + + fileTypeTestCases := []fileTypeTestCase{ + {input: "", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, + {input: "YAML", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, + {input: "yaml", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, + {input: "JSON", output: SecretsFileType(FILE_TYPE_JSON), valid: true, hasTemplate: false}, + {input: "json", output: SecretsFileType(FILE_TYPE_JSON), valid: true, hasTemplate: false}, + {input: "DOTENV", output: SecretsFileType(FILE_TYPE_DOTENV), valid: true, hasTemplate: false}, + {input: "dotenv", output: SecretsFileType(FILE_TYPE_DOTENV), valid: true, hasTemplate: false}, + {input: "BASH", output: SecretsFileType(FILE_TYPE_BASH), valid: true, hasTemplate: false}, + {input: "bash", output: SecretsFileType(FILE_TYPE_BASH), valid: true, hasTemplate: false}, + {input: "PLAINTEXT", output: SecretsFileType(FILE_TYPE_PLAINTEXT), valid: true, hasTemplate: false}, + {input: "plaintext", output: SecretsFileType(FILE_TYPE_PLAINTEXT), valid: true, hasTemplate: false}, + {input: "invalid", output: SecretsFileType(FILE_TYPE_NONE), valid: false, hasTemplate: false}, + {input: "dotenv", output: SecretsFileType(FILE_TYPE_NONE), valid: true, hasTemplate: true}, + {input: "invalid", output: SecretsFileType(FILE_TYPE_NONE), valid: false, hasTemplate: true}, + } + + secretsGroupTestCase := extractSecretsGroupsTestCase{ + description: "file type annotation", + contents: map[string]string{ + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-format.cache": "", + "conjur.org/secret-file-path.cache": "this/relative/file", + }, + } + + resultSecretsGroups := SecretsGroups{ + "cache": { + Secrets: SecretsPaths{ + "test-password": "/test/password", + "test-username": "/test/username", + "url": "/test/url", + }, + FilePath: "/conjur/secrets/this/relative/file", + FilePerms: DEFAULT_FILE_PERMS, + FileTemplate: nil, + }, + } + + for _, tc := range fileTypeTestCases { + secretsGroupTestCase.contents["conjur.org/secret-file-format.cache"] = tc.input + group := resultSecretsGroups["cache"] + group.FileType = tc.output + resultSecretsGroups["cache"] = group + + if tc.hasTemplate { + secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = someValidTemplate + } else { + secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = "" + } + + if tc.valid { + secretsGroupTestCase.assert = assertValidSecretsGroups(resultSecretsGroups) + } else { + secretsGroupTestCase.assert = assertProperError("unknown file format") + } + + t.Run(secretsGroupTestCase.description, func(t *testing.T) { + secretGroups, err := ExtractSecretsGroupsFromAnnotations(secretsGroupTestCase.contents) + secretsGroupTestCase.assert(t, secretGroups, err) + }) + } +} From 4b2a684d4e650f09aea8c29334bf1f7ce1aec0df Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Thu, 23 Sep 2021 16:02:31 -0700 Subject: [PATCH 2/7] CR changes 1 --- pkg/log/messages/error_messages.go | 2 +- .../{push_to_file => pushtofile}/README.md | 0 .../secrets_groups.go | 96 ++++++++++--------- .../secrets_groups_test.go | 60 ++++++------ 4 files changed, 81 insertions(+), 77 deletions(-) rename pkg/secrets/{push_to_file => pushtofile}/README.md (100%) rename pkg/secrets/{push_to_file => pushtofile}/secrets_groups.go (61%) rename pkg/secrets/{push_to_file => pushtofile}/secrets_groups_test.go (69%) diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go index 8d6a4d7e..20fa81ab 100644 --- a/pkg/log/messages/error_messages.go +++ b/pkg/log/messages/error_messages.go @@ -62,4 +62,4 @@ const CSPFK051E string = "CSPFK051E Failed to unmarshal Push-to-File secrets. Re const CSPFK052E string = "CSPFK052E Annotation '%s' must contain relative path without leading '/'" const CSPFK053E string = "CSPFK053E Annotation '%s' specifies unknown file format '%s'" const CSPFK054E string = "CSPFK054E Failed to parse template in annotation '%s'. Reason: %s" -const CSPFK055E string = "CSPFK055E Push-to-File template specified but directory found in annotation '%s'" +const CSPFK055E string = "CSPFK055E Annotation '%s' contains directory instead of file. Destination file name required when template specified." diff --git a/pkg/secrets/push_to_file/README.md b/pkg/secrets/pushtofile/README.md similarity index 100% rename from pkg/secrets/push_to_file/README.md rename to pkg/secrets/pushtofile/README.md diff --git a/pkg/secrets/push_to_file/secrets_groups.go b/pkg/secrets/pushtofile/secrets_groups.go similarity index 61% rename from pkg/secrets/push_to_file/secrets_groups.go rename to pkg/secrets/pushtofile/secrets_groups.go index 73d54f24..f2392610 100644 --- a/pkg/secrets/push_to_file/secrets_groups.go +++ b/pkg/secrets/pushtofile/secrets_groups.go @@ -1,4 +1,4 @@ -package secrets_groups +package pushtofile import ( "fmt" @@ -12,24 +12,26 @@ import ( "gopkg.in/yaml.v3" ) -const SECRETS_GROUP_PREFIX = "conjur.org/conjur-secrets." -const SECRETS_GROUP_POLICY_PATH_PREFIX = "conjur.org/conjur-secrets-policy-path." -const SECRETS_GROUP_FILE_PATH_PREFIX = "conjur.org/secret-file-path." -const SECRETS_GROUP_FILE_TYPE_PREFIX = "conjur.org/secret-file-format." -const SECRETS_GROUP_FILE_TEMPLATE_PREFIX = "conjur.org/secret-file-template." - -const DEFAULT_FILE_PERMS os.FileMode = 0777 -const DEFAULT_FILE_PATH string = "/conjur/secrets/" +const ( + CONJUR_SECRETS_PREFIX = "conjur.org/conjur-secrets." + CONJUR_SECRETS_POLICY_PATH_PREFIX = "conjur.org/conjur-secrets-policy-path." + SECRET_FILE_PATH_PREFIX = "conjur.org/secret-file-path." + SECRET_FILE_FORMAT_PREFIX = "conjur.org/secret-file-format." + SECRET_FILE_TEMPLATE_PREFIX = "conjur.org/secret-file-template." + + DEFAULT_FILE_PERMS os.FileMode = 0777 + DEFAULT_FILE_PATH string = "/conjur/secrets/" +) -type SecretsFileType int +type SecretsFileFormat int const ( - FILE_TYPE_NONE SecretsFileType = iota - FILE_TYPE_YAML - FILE_TYPE_JSON - FILE_TYPE_DOTENV - FILE_TYPE_BASH - FILE_TYPE_PLAINTEXT + FILE_FORMAT_NONE SecretsFileFormat = iota + FILE_FORMAT_YAML + FILE_FORMAT_JSON + FILE_FORMAT_DOTENV + FILE_FORMAT_BASH + FILE_FORMAT_PLAINTEXT ) // SecretsPaths comprises Conjur variable paths for all secrets in a secrets @@ -41,7 +43,7 @@ type SecretsPaths map[string]string type SecretsGroupInfo struct { Secrets SecretsPaths FilePath string - FileType SecretsFileType + FileFormat SecretsFileFormat FilePerms os.FileMode FileTemplate *template.Template } @@ -53,42 +55,42 @@ func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (Secrets secretsGroups := make(SecretsGroups) for annotation := range annotations { - if strings.HasPrefix(annotation, SECRETS_GROUP_PREFIX) { - groupName := strings.TrimPrefix(annotation, SECRETS_GROUP_PREFIX) + if strings.HasPrefix(annotation, CONJUR_SECRETS_PREFIX) { + groupName := strings.TrimPrefix(annotation, CONJUR_SECRETS_PREFIX) - secretsPathPrefix, err := parseSecretsPathPrefix(groupName, annotations[SECRETS_GROUP_POLICY_PATH_PREFIX+groupName]) + secretsPathPrefix, err := parseConjurSecretsPathPrefix(groupName, annotations[CONJUR_SECRETS_POLICY_PATH_PREFIX+groupName]) if err != nil { return nil, err } - secrets, err := parseSecretsPaths(groupName, annotations[SECRETS_GROUP_PREFIX+groupName], secretsPathPrefix) + secrets, err := parseConjurSecretsPaths(groupName, annotations[CONJUR_SECRETS_PREFIX+groupName], secretsPathPrefix) if err != nil { return nil, err } - filePath, err := parseFilePath(groupName, annotations[SECRETS_GROUP_FILE_PATH_PREFIX+groupName]) + filePath, err := parseFilePath(groupName, annotations[SECRET_FILE_PATH_PREFIX+groupName]) if err != nil { return nil, err } - fileType, err := parseFileType(groupName, annotations[SECRETS_GROUP_FILE_TYPE_PREFIX+groupName]) + fileFormat, err := parseFileFormat(groupName, annotations[SECRET_FILE_FORMAT_PREFIX+groupName]) if err != nil { return nil, err } - fileTemplate, err := parseFileTemplate(groupName, annotations[SECRETS_GROUP_FILE_TEMPLATE_PREFIX+groupName]) + fileTemplate, err := parseFileTemplate(groupName, annotations[SECRET_FILE_TEMPLATE_PREFIX+groupName]) if err != nil { return nil, err } if fileTemplate != nil { - fileType = FILE_TYPE_NONE + fileFormat = FILE_FORMAT_NONE } groupInfo := SecretsGroupInfo{ Secrets: secrets, FilePath: filePath, - FileType: fileType, + FileFormat: fileFormat, FilePerms: DEFAULT_FILE_PERMS, FileTemplate: fileTemplate, } @@ -98,12 +100,6 @@ func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (Secrets return nil, err } - /* - if groupInfo.FileTemplate != nil { - groupInfo.FileTemplate.Execute(os.Stdout, groupInfo.Secrets) - } - */ - secretsGroups[groupName] = groupInfo } } @@ -111,7 +107,7 @@ func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (Secrets return secretsGroups, nil } -func parseSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix string) (SecretsPaths, error) { +func parseConjurSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix string) (SecretsPaths, error) { secrets := make(SecretsPaths) unpacked := []interface{}{} @@ -133,7 +129,7 @@ func parseSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix switch val := secret.(type) { case string: { - name := val[strings.LastIndex(val, "/")+1:] + name := path.Base(val) if name == "" { return nil, log.RecordedError(messages.CSPFK051E, "Invalid secret name", groupName) } @@ -165,20 +161,24 @@ func parseSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix return secrets, nil } -func parseSecretsPathPrefix(groupName string, secretsPathPrefix string) (string, error) { +func parseConjurSecretsPathPrefix(groupName string, secretsPathPrefix string) (string, error) { + // By default returns the root policy path '/' secretsPathPrefix = path.Clean("/" + secretsPathPrefix) if secretsPathPrefix == "." || secretsPathPrefix == "/" { return "/", nil } + // Ensure policy path ends with '/' return secretsPathPrefix + "/", nil } func parseFilePath(groupName string, filePath string) (string, error) { + // File path must be relative and can not contain a leading '/' if strings.HasPrefix(filePath, "/") { - return "", log.RecordedError(messages.CSPFK052E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_PATH_PREFIX, groupName)) + return "", log.RecordedError(messages.CSPFK052E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) } + // File path can be a directory (ending in /) or a file name (not ending in /) if strings.HasSuffix(filePath, "/") { filePath = path.Clean(filePath) + "/" } else { @@ -192,22 +192,22 @@ func parseFilePath(groupName string, filePath string) (string, error) { return DEFAULT_FILE_PATH + filePath, nil } -func parseFileType(groupName string, fileType string) (SecretsFileType, error) { - switch strings.ToLower(fileType) { +func parseFileFormat(groupName string, fileFormat string) (SecretsFileFormat, error) { + switch strings.ToLower(fileFormat) { case "": - return FILE_TYPE_YAML, nil + return FILE_FORMAT_YAML, nil case "yaml": - return FILE_TYPE_YAML, nil + return FILE_FORMAT_YAML, nil case "json": - return FILE_TYPE_JSON, nil + return FILE_FORMAT_JSON, nil case "dotenv": - return FILE_TYPE_DOTENV, nil + return FILE_FORMAT_DOTENV, nil case "bash": - return FILE_TYPE_BASH, nil + return FILE_FORMAT_BASH, nil case "plaintext": - return FILE_TYPE_PLAINTEXT, nil + return FILE_FORMAT_PLAINTEXT, nil default: - return FILE_TYPE_NONE, log.RecordedError(messages.CSPFK053E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_TYPE_PREFIX, groupName), fileType) + return FILE_FORMAT_NONE, log.RecordedError(messages.CSPFK053E, fmt.Sprintf("%s%s", SECRET_FILE_FORMAT_PREFIX, groupName), fileFormat) } } @@ -218,16 +218,18 @@ func parseFileTemplate(groupName string, fileTemplate string) (*template.Templat t, err := template.New("FileTemplate").Parse(fileTemplate) if err != nil { - return nil, log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_TEMPLATE_PREFIX, groupName), err.Error()) + return nil, log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRET_FILE_TEMPLATE_PREFIX, groupName), err.Error()) } return t, nil } func validateGroupInfo(groupName string, groupInfo SecretsGroupInfo) error { + // If a template is specified, then a secrets + // file name is required, not just a directory if groupInfo.FileTemplate != nil { if strings.HasSuffix(groupInfo.FilePath, "/") { - return log.RecordedError(messages.CSPFK055E, fmt.Sprintf("%s%s", SECRETS_GROUP_FILE_PATH_PREFIX, groupName)) + return log.RecordedError(messages.CSPFK055E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) } } diff --git a/pkg/secrets/push_to_file/secrets_groups_test.go b/pkg/secrets/pushtofile/secrets_groups_test.go similarity index 69% rename from pkg/secrets/push_to_file/secrets_groups_test.go rename to pkg/secrets/pushtofile/secrets_groups_test.go index 657fb6f0..1271d1ff 100644 --- a/pkg/secrets/push_to_file/secrets_groups_test.go +++ b/pkg/secrets/pushtofile/secrets_groups_test.go @@ -1,4 +1,4 @@ -package secrets_groups +package pushtofile import ( "testing" @@ -6,13 +6,15 @@ import ( "github.com/stretchr/testify/assert" ) +type assertFunc func(*testing.T, SecretsGroups, error) + type extractSecretsGroupsTestCase struct { description string contents map[string]string - assert func(t *testing.T, result SecretsGroups, err error) + assert assertFunc } -func assertValidSecretsGroups(expected SecretsGroups) func(*testing.T, SecretsGroups, error) { +func assertExpectedSecretsGroups(expected SecretsGroups) assertFunc { return func(t *testing.T, result SecretsGroups, err error) { if !assert.NoError(t, err) { return @@ -25,7 +27,7 @@ func assertValidSecretsGroups(expected SecretsGroups) func(*testing.T, SecretsGr assert.Equal(t, expectedVal.Secrets, resultVal.Secrets) assert.Equal(t, expectedVal.FilePath, resultVal.FilePath) - assert.Equal(t, expectedVal.FileType, resultVal.FileType) + assert.Equal(t, expectedVal.FileFormat, resultVal.FileFormat) assert.Equal(t, expectedVal.FilePerms, resultVal.FilePerms) if expectedVal.FileTemplate != nil { @@ -35,7 +37,7 @@ func assertValidSecretsGroups(expected SecretsGroups) func(*testing.T, SecretsGr } } -func assertProperError(expectedErr string) func(*testing.T, SecretsGroups, error) { +func assertProperError(expectedErr string) assertFunc { return func(t *testing.T, result SecretsGroups, err error) { assert.Nil(t, result) assert.Contains(t, err.Error(), expectedErr) @@ -60,7 +62,7 @@ var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ "conjur.org/secret-file-path.cache": "this/relative/path", "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", }, - assert: assertValidSecretsGroups( + assert: assertExpectedSecretsGroups( SecretsGroups{ "cache": { Secrets: SecretsPaths{ @@ -69,7 +71,7 @@ var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ "url": "/some/policy/path/test/url", }, FilePath: "/conjur/secrets/this/relative/path", - FileType: SecretsFileType(FILE_TYPE_YAML), + FileFormat: SecretsFileFormat(FILE_FORMAT_YAML), FilePerms: DEFAULT_FILE_PERMS, FileTemplate: nil, }, @@ -91,7 +93,7 @@ var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ "conjur.org/secret-file-path.cache": "this/relative/directory/", "conjur.org/secret-file-template.cache": someValidTemplate, }, - assert: assertProperError("template specified but directory found"), + assert: assertProperError("contains directory instead of file"), }, } @@ -105,29 +107,29 @@ func TestExtractSecretsGroupsFromAnnotations(t *testing.T) { } } -func TestExtractFileTypeFromAnnotations(t *testing.T) { - type fileTypeTestCase struct { +func TestExtractFileFormatFromAnnotations(t *testing.T) { + type fileFormatTestCase struct { input string - output SecretsFileType + output SecretsFileFormat valid bool hasTemplate bool } - fileTypeTestCases := []fileTypeTestCase{ - {input: "", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, - {input: "YAML", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, - {input: "yaml", output: SecretsFileType(FILE_TYPE_YAML), valid: true, hasTemplate: false}, - {input: "JSON", output: SecretsFileType(FILE_TYPE_JSON), valid: true, hasTemplate: false}, - {input: "json", output: SecretsFileType(FILE_TYPE_JSON), valid: true, hasTemplate: false}, - {input: "DOTENV", output: SecretsFileType(FILE_TYPE_DOTENV), valid: true, hasTemplate: false}, - {input: "dotenv", output: SecretsFileType(FILE_TYPE_DOTENV), valid: true, hasTemplate: false}, - {input: "BASH", output: SecretsFileType(FILE_TYPE_BASH), valid: true, hasTemplate: false}, - {input: "bash", output: SecretsFileType(FILE_TYPE_BASH), valid: true, hasTemplate: false}, - {input: "PLAINTEXT", output: SecretsFileType(FILE_TYPE_PLAINTEXT), valid: true, hasTemplate: false}, - {input: "plaintext", output: SecretsFileType(FILE_TYPE_PLAINTEXT), valid: true, hasTemplate: false}, - {input: "invalid", output: SecretsFileType(FILE_TYPE_NONE), valid: false, hasTemplate: false}, - {input: "dotenv", output: SecretsFileType(FILE_TYPE_NONE), valid: true, hasTemplate: true}, - {input: "invalid", output: SecretsFileType(FILE_TYPE_NONE), valid: false, hasTemplate: true}, + fileFormatTestCases := []fileFormatTestCase{ + {input: "", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, + {input: "YAML", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, + {input: "yaml", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, + {input: "JSON", output: SecretsFileFormat(FILE_FORMAT_JSON), valid: true, hasTemplate: false}, + {input: "json", output: SecretsFileFormat(FILE_FORMAT_JSON), valid: true, hasTemplate: false}, + {input: "DOTENV", output: SecretsFileFormat(FILE_FORMAT_DOTENV), valid: true, hasTemplate: false}, + {input: "dotenv", output: SecretsFileFormat(FILE_FORMAT_DOTENV), valid: true, hasTemplate: false}, + {input: "BASH", output: SecretsFileFormat(FILE_FORMAT_BASH), valid: true, hasTemplate: false}, + {input: "bash", output: SecretsFileFormat(FILE_FORMAT_BASH), valid: true, hasTemplate: false}, + {input: "PLAINTEXT", output: SecretsFileFormat(FILE_FORMAT_PLAINTEXT), valid: true, hasTemplate: false}, + {input: "plaintext", output: SecretsFileFormat(FILE_FORMAT_PLAINTEXT), valid: true, hasTemplate: false}, + {input: "invalid", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: false, hasTemplate: false}, + {input: "dotenv", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: true, hasTemplate: true}, + {input: "invalid", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: false, hasTemplate: true}, } secretsGroupTestCase := extractSecretsGroupsTestCase{ @@ -152,10 +154,10 @@ func TestExtractFileTypeFromAnnotations(t *testing.T) { }, } - for _, tc := range fileTypeTestCases { + for _, tc := range fileFormatTestCases { secretsGroupTestCase.contents["conjur.org/secret-file-format.cache"] = tc.input group := resultSecretsGroups["cache"] - group.FileType = tc.output + group.FileFormat = tc.output resultSecretsGroups["cache"] = group if tc.hasTemplate { @@ -165,7 +167,7 @@ func TestExtractFileTypeFromAnnotations(t *testing.T) { } if tc.valid { - secretsGroupTestCase.assert = assertValidSecretsGroups(resultSecretsGroups) + secretsGroupTestCase.assert = assertExpectedSecretsGroups(resultSecretsGroups) } else { secretsGroupTestCase.assert = assertProperError("unknown file format") } From 15502a868c877b48a916308a022c3937d90d5d36 Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Thu, 23 Sep 2021 16:19:22 -0700 Subject: [PATCH 3/7] Remove template parsing --- pkg/log/messages/error_messages.go | 3 +-- pkg/secrets/pushtofile/secrets_groups.go | 27 ++++--------------- pkg/secrets/pushtofile/secrets_groups_test.go | 24 +++++------------ 3 files changed, 13 insertions(+), 41 deletions(-) diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go index 20fa81ab..5bd0d04c 100644 --- a/pkg/log/messages/error_messages.go +++ b/pkg/log/messages/error_messages.go @@ -61,5 +61,4 @@ const CSPFK039E string = "CSPFK039E Secrets Provider for Kubernetes failed to up const CSPFK051E string = "CSPFK051E Failed to unmarshal Push-to-File secrets. Reason: %s in group '%s'" const CSPFK052E string = "CSPFK052E Annotation '%s' must contain relative path without leading '/'" const CSPFK053E string = "CSPFK053E Annotation '%s' specifies unknown file format '%s'" -const CSPFK054E string = "CSPFK054E Failed to parse template in annotation '%s'. Reason: %s" -const CSPFK055E string = "CSPFK055E Annotation '%s' contains directory instead of file. Destination file name required when template specified." +const CSPFK054E string = "CSPFK055E Annotation '%s' contains directory instead of file. Destination file name required when template specified." diff --git a/pkg/secrets/pushtofile/secrets_groups.go b/pkg/secrets/pushtofile/secrets_groups.go index f2392610..db934c66 100644 --- a/pkg/secrets/pushtofile/secrets_groups.go +++ b/pkg/secrets/pushtofile/secrets_groups.go @@ -5,7 +5,6 @@ import ( "os" "path" "strings" - "text/template" "github.com/cyberark/conjur-authn-k8s-client/pkg/log" "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" @@ -45,7 +44,7 @@ type SecretsGroupInfo struct { FilePath string FileFormat SecretsFileFormat FilePerms os.FileMode - FileTemplate *template.Template + FileTemplate string } // SecretsGroups comprises secrets mapping info for all secret groups @@ -78,12 +77,9 @@ func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (Secrets return nil, err } - fileTemplate, err := parseFileTemplate(groupName, annotations[SECRET_FILE_TEMPLATE_PREFIX+groupName]) - if err != nil { - return nil, err - } + fileTemplate := annotations[SECRET_FILE_TEMPLATE_PREFIX+groupName] - if fileTemplate != nil { + if fileTemplate != "" { fileFormat = FILE_FORMAT_NONE } @@ -211,25 +207,12 @@ func parseFileFormat(groupName string, fileFormat string) (SecretsFileFormat, er } } -func parseFileTemplate(groupName string, fileTemplate string) (*template.Template, error) { - if fileTemplate == "" { - return nil, nil - } - - t, err := template.New("FileTemplate").Parse(fileTemplate) - if err != nil { - return nil, log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRET_FILE_TEMPLATE_PREFIX, groupName), err.Error()) - } - - return t, nil -} - func validateGroupInfo(groupName string, groupInfo SecretsGroupInfo) error { // If a template is specified, then a secrets // file name is required, not just a directory - if groupInfo.FileTemplate != nil { + if groupInfo.FileTemplate != "" { if strings.HasSuffix(groupInfo.FilePath, "/") { - return log.RecordedError(messages.CSPFK055E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) + return log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) } } diff --git a/pkg/secrets/pushtofile/secrets_groups_test.go b/pkg/secrets/pushtofile/secrets_groups_test.go index 1271d1ff..fc49a6be 100644 --- a/pkg/secrets/pushtofile/secrets_groups_test.go +++ b/pkg/secrets/pushtofile/secrets_groups_test.go @@ -20,20 +20,7 @@ func assertExpectedSecretsGroups(expected SecretsGroups) assertFunc { return } - assert.Equal(t, len(expected), len(result)) - for expectedKey, expectedVal := range expected { - resultVal, ok := result[expectedKey] - assert.True(t, ok) - - assert.Equal(t, expectedVal.Secrets, resultVal.Secrets) - assert.Equal(t, expectedVal.FilePath, resultVal.FilePath) - assert.Equal(t, expectedVal.FileFormat, resultVal.FileFormat) - assert.Equal(t, expectedVal.FilePerms, resultVal.FilePerms) - - if expectedVal.FileTemplate != nil { - assert.NotNil(t, resultVal.FileTemplate) - } - } + assert.Equal(t, expected, result) } } @@ -73,7 +60,7 @@ var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ FilePath: "/conjur/secrets/this/relative/path", FileFormat: SecretsFileFormat(FILE_FORMAT_YAML), FilePerms: DEFAULT_FILE_PERMS, - FileTemplate: nil, + FileTemplate: "", }, }, ), @@ -150,7 +137,7 @@ func TestExtractFileFormatFromAnnotations(t *testing.T) { }, FilePath: "/conjur/secrets/this/relative/file", FilePerms: DEFAULT_FILE_PERMS, - FileTemplate: nil, + FileTemplate: "", }, } @@ -158,12 +145,13 @@ func TestExtractFileFormatFromAnnotations(t *testing.T) { secretsGroupTestCase.contents["conjur.org/secret-file-format.cache"] = tc.input group := resultSecretsGroups["cache"] group.FileFormat = tc.output - resultSecretsGroups["cache"] = group if tc.hasTemplate { secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = someValidTemplate + group.FileTemplate = someValidTemplate } else { secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = "" + group.FileTemplate = "" } if tc.valid { @@ -172,6 +160,8 @@ func TestExtractFileFormatFromAnnotations(t *testing.T) { secretsGroupTestCase.assert = assertProperError("unknown file format") } + resultSecretsGroups["cache"] = group + t.Run(secretsGroupTestCase.description, func(t *testing.T) { secretGroups, err := ExtractSecretsGroupsFromAnnotations(secretsGroupTestCase.contents) secretsGroupTestCase.assert(t, secretGroups, err) From 3fe4b78ed587c9f96c0676a21376e20c8cc7f9f6 Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Tue, 28 Sep 2021 13:33:29 -0700 Subject: [PATCH 4/7] Revert good code --- pkg/log/messages/error_messages.go | 6 +- pkg/secrets/pushtofile/secret_groups.go | 90 +++++++ pkg/secrets/pushtofile/secret_groups_test.go | 140 +++++++++++ pkg/secrets/pushtofile/secret_spec.go | 61 +++++ pkg/secrets/pushtofile/secret_spec_test.go | 79 +++++++ pkg/secrets/pushtofile/secrets_groups.go | 220 ------------------ pkg/secrets/pushtofile/secrets_groups_test.go | 170 -------------- 7 files changed, 372 insertions(+), 394 deletions(-) create mode 100644 pkg/secrets/pushtofile/secret_groups.go create mode 100644 pkg/secrets/pushtofile/secret_groups_test.go create mode 100644 pkg/secrets/pushtofile/secret_spec.go create mode 100644 pkg/secrets/pushtofile/secret_spec_test.go delete mode 100644 pkg/secrets/pushtofile/secrets_groups.go delete mode 100644 pkg/secrets/pushtofile/secrets_groups_test.go diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go index 5bd0d04c..938f182b 100644 --- a/pkg/log/messages/error_messages.go +++ b/pkg/log/messages/error_messages.go @@ -58,7 +58,5 @@ const CSPFK038E string = "CSPFK038E Retransmission backoff exhausted" const CSPFK039E string = "CSPFK039E Secrets Provider for Kubernetes failed to update Kubernetes Secrets" // Push to File -const CSPFK051E string = "CSPFK051E Failed to unmarshal Push-to-File secrets. Reason: %s in group '%s'" -const CSPFK052E string = "CSPFK052E Annotation '%s' must contain relative path without leading '/'" -const CSPFK053E string = "CSPFK053E Annotation '%s' specifies unknown file format '%s'" -const CSPFK054E string = "CSPFK055E Annotation '%s' contains directory instead of file. Destination file name required when template specified." +const CSPFK051E string = "CSPFK051E Failed to unmarshal Push-to-File secrets. Reason: %s" +const CSPFK052E string = "CSPFK052E Unknown file format '%s'" diff --git a/pkg/secrets/pushtofile/secret_groups.go b/pkg/secrets/pushtofile/secret_groups.go new file mode 100644 index 00000000..45890b5d --- /dev/null +++ b/pkg/secrets/pushtofile/secret_groups.go @@ -0,0 +1,90 @@ +package pushtofile + +import ( + "os" + "strings" + + "github.com/cyberark/conjur-authn-k8s-client/pkg/log" + "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" +) + +const ( + conjurSecretsPrefix = "conjur.org/conjur-secrets." + conjurSecretsPolicyPathPrefix = "conjur.org/conjur-secrets-policy-path." + secretFilePathPrefix = "conjur.org/secret-file-path." + secretFileFormatPrefix = "conjur.org/secret-file-format." + secretFileTemplatePrefix = "conjur.org/secret-file-template." + + defaultFilePerms os.FileMode = 0777 +) + +// SecretGroup comprises secrets mapping information for a given secrets +// group. +type SecretGroup struct { + Label string + FilePath string + FileTemplate string + ConjurSecretPathPrefix string + SecretSpecs []SecretSpec + FileFormat string + FilePerms os.FileMode +} + +// SecretGroups comprises secrets mapping info for all secret groups +type SecretGroups []SecretGroup + +func NewSecretGroupsFromAnnotations(annotations map[string]string) (SecretGroups, error) { + secretsGroups := []SecretGroup{} + + for annotation := range annotations { + if strings.HasPrefix(annotation, conjurSecretsPrefix) { + groupName := strings.TrimPrefix(annotation, conjurSecretsPrefix) + conjurSecretPathPrefix := annotations[conjurSecretsPolicyPathPrefix+groupName] + filePath := annotations[secretFilePathPrefix+groupName] + fileTemplate := annotations[secretFileTemplatePrefix+groupName] + + fileFormat, err := parseFileFormat(annotations[secretFileFormatPrefix+groupName]) + if err != nil { + return nil, err + } + + secrets, err := NewSecretSpecs([]byte(annotations[conjurSecretsPrefix+groupName])) + if err != nil { + return nil, err + } + + group := SecretGroup{ + Label: groupName, + FilePath: filePath, + FileTemplate: fileTemplate, + ConjurSecretPathPrefix: conjurSecretPathPrefix, + SecretSpecs: secrets, + FileFormat: fileFormat, + FilePerms: defaultFilePerms, + } + + secretsGroups = append(secretsGroups, group) + } + } + + return secretsGroups, nil +} + +func parseFileFormat(fileFormat string) (string, error) { + switch fileFormat { + case "yaml": + fallthrough + case "json": + fallthrough + case "dotenv": + fallthrough + case "bash": + fallthrough + case "plaintext": + return fileFormat, nil + case "": + return "yaml", nil + default: + return "", log.RecordedError(messages.CSPFK052E, fileFormat) + } +} diff --git a/pkg/secrets/pushtofile/secret_groups_test.go b/pkg/secrets/pushtofile/secret_groups_test.go new file mode 100644 index 00000000..a0256f9d --- /dev/null +++ b/pkg/secrets/pushtofile/secret_groups_test.go @@ -0,0 +1,140 @@ +package pushtofile + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type assertFunc func(*testing.T, SecretGroups, error) + +type extractSecretGroupsTestCase struct { + description string + contents map[string]string + assert assertFunc +} + +func assertExpectedSecretGroups(expected SecretGroups) assertFunc { + return func(t *testing.T, result SecretGroups, err error) { + if !assert.NoError(t, err) { + return + } + + assert.Equal(t, expected, result) + } +} + +const someValidSecrets = "- test/url\n- test-password: test/password\n- test-username: test/username\n" + +var extractSecretGroupsTestCases = []extractSecretGroupsTestCase{ + { + description: "valid annotations map", + contents: map[string]string{ + "conjur.org/authn-identity": "host/conjur/authn-k8s/cluster/apps/inventory-api", + "conjur.org/container-mode": "application", + "conjur.org/secret-destination": "file", + "conjur.org/k8s-secret": "- k8s-secret-1\n- k8s-secret-2\n", + "conjur.org/retry-count-limit": "10", + "conjur.org/retry-interval-sec": "5", + "conjur.org/debug-logging": "true", + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "this/relative/path", + "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", + "conjur.org/secret-file-template.cache": "some-template", + "conjur.org/secret-file-format.cache": "yaml", + }, + assert: assertExpectedSecretGroups( + SecretGroups{ + { + Label: "cache", + FilePath: "this/relative/path", + FileTemplate: "some-template", + ConjurSecretPathPrefix: "some/policy/path", + SecretSpecs: []SecretSpec{ + {Id: "test/url", Alias: "url"}, + {Id: "test/password", Alias: "test-password"}, + {Id: "test/username", Alias: "test-username"}, + }, + FileFormat: "yaml", + FilePerms: defaultFilePerms, + }, + }, + ), + }, + { + description: "valid annotations map", + contents: map[string]string{ + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "this/relative/path", + "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", + "conjur.org/secret-file-template.cache": "some-template", + "conjur.org/secret-file-format.cache": "yaml", + }, + assert: assertExpectedSecretGroups( + SecretGroups{ + { + Label: "cache", + FilePath: "this/relative/path", + FileTemplate: "some-template", + ConjurSecretPathPrefix: "some/policy/path", + SecretSpecs: []SecretSpec{ + {Id: "test/url", Alias: "url"}, + {Id: "test/password", Alias: "test-password"}, + {Id: "test/username", Alias: "test-username"}, + }, + FileFormat: "yaml", + FilePerms: defaultFilePerms, + }, + }, + ), + }, +} + +func TestNewSecretGroupsFromAnnotations(t *testing.T) { + for _, tc := range extractSecretGroupsTestCases { + t.Run(tc.description, func(t *testing.T) { + secretGroups, err := NewSecretGroupsFromAnnotations(tc.contents) + + tc.assert(t, secretGroups, err) + }) + } +} + +func TestExtractFileFormatFromAnnotations(t *testing.T) { + type fileFormatTestCase struct { + input string + output string + valid bool + } + + fileFormatTestCases := []fileFormatTestCase{ + {input: "", output: "yaml", valid: true}, + {input: "yaml", output: "yaml", valid: true}, + {input: "json", output: "json", valid: true}, + {input: "dotenv", output: "dotenv", valid: true}, + {input: "bash", output: "bash", valid: true}, + {input: "plaintext", output: "plaintext", valid: true}, + {input: "invalid", output: "", valid: false}, + } + + someValidAnnotationsMap := map[string]string{ + "conjur.org/conjur-secrets.cache": someValidSecrets, + "conjur.org/secret-file-path.cache": "some/file/name", + } + + for _, tc := range fileFormatTestCases { + someValidAnnotationsMap["conjur.org/secret-file-format.cache"] = tc.input + + t.Run("parse file format", func(t *testing.T) { + secretGroups, err := NewSecretGroupsFromAnnotations(someValidAnnotationsMap) + + if tc.valid { + assert.NoError(t, err) + assert.Equal(t, tc.output, secretGroups[0].FileFormat) + } else { + assert.Nil(t, secretGroups) + assert.Contains(t, err.Error(), "Unknown file format") + } + }) + } +} diff --git a/pkg/secrets/pushtofile/secret_spec.go b/pkg/secrets/pushtofile/secret_spec.go new file mode 100644 index 00000000..05a14a01 --- /dev/null +++ b/pkg/secrets/pushtofile/secret_spec.go @@ -0,0 +1,61 @@ +package pushtofile + +import ( + "fmt" + "strings" + + "github.com/cyberark/conjur-authn-k8s-client/pkg/log" + "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" + "gopkg.in/yaml.v3" +) + +type SecretSpec struct { + Id string + Alias string +} + +func (t SecretSpec) MarshalYAML() (interface{}, error) { + return map[string]string{t.Alias: t.Id}, nil +} + +// UnmarshalYAML is a custom unmarshaller for SecretSpec that allows it to unmarshal +// from different yaml types by trying each one +func (t *SecretSpec) UnmarshalYAML(unmarshal func(v interface{}) error) error { + // LITERAL + var literalValue string + err := unmarshal(&literalValue) + if err == nil { + t.Id = literalValue + t.Alias = literalValue[strings.LastIndex(literalValue, "/")+1:] + return nil + } + + // MAP + var mapValue map[string]string + err = unmarshal(&mapValue) + if err == nil { + if len(mapValue) != 1 { + return log.RecordedError(messages.CSPFK051E, "expected single key-value pair for secret specification") + } + + for k, v := range mapValue { + t.Id = v + t.Alias = k + } + + return nil + } + + // ELSE + return log.RecordedError(messages.CSPFK051E, "unrecognized format for secret spec") +} + +func NewSecretSpecs(raw []byte) ([]SecretSpec, error) { + var secretSpecs []SecretSpec + err := yaml.Unmarshal(raw, &secretSpecs) + if err != nil { + return nil, log.RecordedError(messages.CSPFK051E, fmt.Sprintf("failed to parse yaml list: %v", err)) + } + + return secretSpecs, nil +} diff --git a/pkg/secrets/pushtofile/secret_spec_test.go b/pkg/secrets/pushtofile/secret_spec_test.go new file mode 100644 index 00000000..dacbfee1 --- /dev/null +++ b/pkg/secrets/pushtofile/secret_spec_test.go @@ -0,0 +1,79 @@ +package pushtofile + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type secretsSpecTestCase struct { + description string + contents string + assert func(t *testing.T, result []SecretSpec, err error) +} + +func assertGoodSecretSpecs(expectedResult []SecretSpec) func(*testing.T, []SecretSpec, error) { + return func(t *testing.T, result []SecretSpec, err error) { + if !assert.NoError(t, err) { + return + } + + assert.Equal( + t, + expectedResult, + result, + ) + } +} + +var secretsSpecTestCases = []secretsSpecTestCase{ + { + description: "valid example", + contents: ` +- dev/openshift/api-url +- admin-password: dev/openshift/password +`, + assert: assertGoodSecretSpecs( + []SecretSpec{ + { + Id: "dev/openshift/api-url", + Alias: "api-url", + }, + { + Id: "dev/openshift/password", + Alias: "admin-password", + }, + }, + ), + }, + { + description: "malformed not a list", + contents: ` +admin-password: dev/openshift/password +another-password: dev/openshift/password +`, + assert: func(t *testing.T, result []SecretSpec, err error) { + assert.Contains(t, err.Error(), "failed to parse yaml list") + }, + }, + { + description: "malformed multiple key-values in one entry", + contents: ` +- dev/openshift/api-url +- admin-password: dev/openshift/password + another-admin-password: dev/openshift/password +`, + assert: func(t *testing.T, result []SecretSpec, err error) { + assert.Contains(t, err.Error(), "expected single key-value pair for secret specification") + }, + }, +} + +func TestNewSecretSpecs(t *testing.T) { + for _, tc := range secretsSpecTestCases { + t.Run(tc.description, func(t *testing.T) { + secretsSpecs, err := NewSecretSpecs([]byte(tc.contents)) + tc.assert(t, secretsSpecs, err) + }) + } +} diff --git a/pkg/secrets/pushtofile/secrets_groups.go b/pkg/secrets/pushtofile/secrets_groups.go deleted file mode 100644 index db934c66..00000000 --- a/pkg/secrets/pushtofile/secrets_groups.go +++ /dev/null @@ -1,220 +0,0 @@ -package pushtofile - -import ( - "fmt" - "os" - "path" - "strings" - - "github.com/cyberark/conjur-authn-k8s-client/pkg/log" - "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" - "gopkg.in/yaml.v3" -) - -const ( - CONJUR_SECRETS_PREFIX = "conjur.org/conjur-secrets." - CONJUR_SECRETS_POLICY_PATH_PREFIX = "conjur.org/conjur-secrets-policy-path." - SECRET_FILE_PATH_PREFIX = "conjur.org/secret-file-path." - SECRET_FILE_FORMAT_PREFIX = "conjur.org/secret-file-format." - SECRET_FILE_TEMPLATE_PREFIX = "conjur.org/secret-file-template." - - DEFAULT_FILE_PERMS os.FileMode = 0777 - DEFAULT_FILE_PATH string = "/conjur/secrets/" -) - -type SecretsFileFormat int - -const ( - FILE_FORMAT_NONE SecretsFileFormat = iota - FILE_FORMAT_YAML - FILE_FORMAT_JSON - FILE_FORMAT_DOTENV - FILE_FORMAT_BASH - FILE_FORMAT_PLAINTEXT -) - -// SecretsPaths comprises Conjur variable paths for all secrets in a secrets -// group, indexed by secret name. -type SecretsPaths map[string]string - -// SecretsGroupInfo comprises secrets mapping information for a given secrets -// group. -type SecretsGroupInfo struct { - Secrets SecretsPaths - FilePath string - FileFormat SecretsFileFormat - FilePerms os.FileMode - FileTemplate string -} - -// SecretsGroups comprises secrets mapping info for all secret groups -type SecretsGroups map[string]SecretsGroupInfo - -func ExtractSecretsGroupsFromAnnotations(annotations map[string]string) (SecretsGroups, error) { - secretsGroups := make(SecretsGroups) - - for annotation := range annotations { - if strings.HasPrefix(annotation, CONJUR_SECRETS_PREFIX) { - groupName := strings.TrimPrefix(annotation, CONJUR_SECRETS_PREFIX) - - secretsPathPrefix, err := parseConjurSecretsPathPrefix(groupName, annotations[CONJUR_SECRETS_POLICY_PATH_PREFIX+groupName]) - if err != nil { - return nil, err - } - - secrets, err := parseConjurSecretsPaths(groupName, annotations[CONJUR_SECRETS_PREFIX+groupName], secretsPathPrefix) - if err != nil { - return nil, err - } - - filePath, err := parseFilePath(groupName, annotations[SECRET_FILE_PATH_PREFIX+groupName]) - if err != nil { - return nil, err - } - - fileFormat, err := parseFileFormat(groupName, annotations[SECRET_FILE_FORMAT_PREFIX+groupName]) - if err != nil { - return nil, err - } - - fileTemplate := annotations[SECRET_FILE_TEMPLATE_PREFIX+groupName] - - if fileTemplate != "" { - fileFormat = FILE_FORMAT_NONE - } - - groupInfo := SecretsGroupInfo{ - Secrets: secrets, - FilePath: filePath, - FileFormat: fileFormat, - FilePerms: DEFAULT_FILE_PERMS, - FileTemplate: fileTemplate, - } - - err = validateGroupInfo(groupName, groupInfo) - if err != nil { - return nil, err - } - - secretsGroups[groupName] = groupInfo - } - } - - return secretsGroups, nil -} - -func parseConjurSecretsPaths(groupName string, secretsPaths string, secretsPathsPrefix string) (SecretsPaths, error) { - secrets := make(SecretsPaths) - - unpacked := []interface{}{} - if err := yaml.Unmarshal([]byte(secretsPaths), &unpacked); err != nil { - return nil, log.RecordedError(messages.CSPFK051E, err.Error(), groupName) - } - - insertSecret := func(name string, secretPath string) error { - secretPath = path.Clean(secretPath) - if secretPath == "." { - return log.RecordedError(messages.CSPFK051E, "Invalid secret path", groupName) - } - secrets[name] = secretsPathsPrefix + secretPath - - return nil - } - - for _, secret := range unpacked { - switch val := secret.(type) { - case string: - { - name := path.Base(val) - if name == "" { - return nil, log.RecordedError(messages.CSPFK051E, "Invalid secret name", groupName) - } - - if err := insertSecret(name, val); err != nil { - return nil, err - } - } - - case map[string]interface{}: - { - for alias, secretPath := range val { - secretPath, _ := secretPath.(string) - if err := insertSecret(alias, secretPath); err != nil { - return nil, err - } - } - } - - default: - return nil, log.RecordedError(messages.CSPFK051E, "Unknown secret format", groupName) - } - } - - if len(secrets) == 0 { - return nil, log.RecordedError(messages.CSPFK051E, "No valid secrets found", groupName) - } - - return secrets, nil -} - -func parseConjurSecretsPathPrefix(groupName string, secretsPathPrefix string) (string, error) { - // By default returns the root policy path '/' - secretsPathPrefix = path.Clean("/" + secretsPathPrefix) - if secretsPathPrefix == "." || secretsPathPrefix == "/" { - return "/", nil - } - - // Ensure policy path ends with '/' - return secretsPathPrefix + "/", nil -} - -func parseFilePath(groupName string, filePath string) (string, error) { - // File path must be relative and can not contain a leading '/' - if strings.HasPrefix(filePath, "/") { - return "", log.RecordedError(messages.CSPFK052E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) - } - - // File path can be a directory (ending in /) or a file name (not ending in /) - if strings.HasSuffix(filePath, "/") { - filePath = path.Clean(filePath) + "/" - } else { - filePath = path.Clean(filePath) - } - - if filePath == "." { - return DEFAULT_FILE_PATH, nil - } - - return DEFAULT_FILE_PATH + filePath, nil -} - -func parseFileFormat(groupName string, fileFormat string) (SecretsFileFormat, error) { - switch strings.ToLower(fileFormat) { - case "": - return FILE_FORMAT_YAML, nil - case "yaml": - return FILE_FORMAT_YAML, nil - case "json": - return FILE_FORMAT_JSON, nil - case "dotenv": - return FILE_FORMAT_DOTENV, nil - case "bash": - return FILE_FORMAT_BASH, nil - case "plaintext": - return FILE_FORMAT_PLAINTEXT, nil - default: - return FILE_FORMAT_NONE, log.RecordedError(messages.CSPFK053E, fmt.Sprintf("%s%s", SECRET_FILE_FORMAT_PREFIX, groupName), fileFormat) - } -} - -func validateGroupInfo(groupName string, groupInfo SecretsGroupInfo) error { - // If a template is specified, then a secrets - // file name is required, not just a directory - if groupInfo.FileTemplate != "" { - if strings.HasSuffix(groupInfo.FilePath, "/") { - return log.RecordedError(messages.CSPFK054E, fmt.Sprintf("%s%s", SECRET_FILE_PATH_PREFIX, groupName)) - } - } - - return nil -} diff --git a/pkg/secrets/pushtofile/secrets_groups_test.go b/pkg/secrets/pushtofile/secrets_groups_test.go deleted file mode 100644 index fc49a6be..00000000 --- a/pkg/secrets/pushtofile/secrets_groups_test.go +++ /dev/null @@ -1,170 +0,0 @@ -package pushtofile - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -type assertFunc func(*testing.T, SecretsGroups, error) - -type extractSecretsGroupsTestCase struct { - description string - contents map[string]string - assert assertFunc -} - -func assertExpectedSecretsGroups(expected SecretsGroups) assertFunc { - return func(t *testing.T, result SecretsGroups, err error) { - if !assert.NoError(t, err) { - return - } - - assert.Equal(t, expected, result) - } -} - -func assertProperError(expectedErr string) assertFunc { - return func(t *testing.T, result SecretsGroups, err error) { - assert.Nil(t, result) - assert.Contains(t, err.Error(), expectedErr) - } -} - -const someValidSecrets = "- test/url\n- test-password: test/password\n- test-username: test/username\n" -const someValidTemplate = "\"cache\": {\n\t\"url\": {{ .url }},\n\t\"admin-password\": {{ index . \"test-password\" }},\n\t\"admin-username\": {{ index . \"test-username\" }},\n\t\"port\": 123456,\n}" - -var extractSecretsGroupsTestCases = []extractSecretsGroupsTestCase{ - { - description: "valid annotations map", - contents: map[string]string{ - "conjur.org/authn-identity": "host/conjur/authn-k8s/cluster/apps/inventory-api", - "conjur.org/container-mode": "application", - "conjur.org/secrets-destination": "file", - "conjur.org/k8s-secrets": "- k8s-secret-1\n- k8s-secret-2\n", - "conjur.org/retry-count-limit": "10", - "conjur.org/retry-interval-sec": "5", - "conjur.org/debug-logging": "true", - "conjur.org/conjur-secrets.cache": someValidSecrets, - "conjur.org/secret-file-path.cache": "this/relative/path", - "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", - }, - assert: assertExpectedSecretsGroups( - SecretsGroups{ - "cache": { - Secrets: SecretsPaths{ - "test-password": "/some/policy/path/test/password", - "test-username": "/some/policy/path/test/username", - "url": "/some/policy/path/test/url", - }, - FilePath: "/conjur/secrets/this/relative/path", - FileFormat: SecretsFileFormat(FILE_FORMAT_YAML), - FilePerms: DEFAULT_FILE_PERMS, - FileTemplate: "", - }, - }, - ), - }, - { - description: "invalid file path", - contents: map[string]string{ - "conjur.org/conjur-secrets.cache": someValidSecrets, - "conjur.org/secret-file-path.cache": "/this/absolute/path", - }, - assert: assertProperError("must contain relative path"), - }, - { - description: "invalid file path with template specified", - contents: map[string]string{ - "conjur.org/conjur-secrets.cache": someValidSecrets, - "conjur.org/secret-file-path.cache": "this/relative/directory/", - "conjur.org/secret-file-template.cache": someValidTemplate, - }, - assert: assertProperError("contains directory instead of file"), - }, -} - -func TestExtractSecretsGroupsFromAnnotations(t *testing.T) { - for _, tc := range extractSecretsGroupsTestCases { - t.Run(tc.description, func(t *testing.T) { - secretGroups, err := ExtractSecretsGroupsFromAnnotations(tc.contents) - - tc.assert(t, secretGroups, err) - }) - } -} - -func TestExtractFileFormatFromAnnotations(t *testing.T) { - type fileFormatTestCase struct { - input string - output SecretsFileFormat - valid bool - hasTemplate bool - } - - fileFormatTestCases := []fileFormatTestCase{ - {input: "", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, - {input: "YAML", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, - {input: "yaml", output: SecretsFileFormat(FILE_FORMAT_YAML), valid: true, hasTemplate: false}, - {input: "JSON", output: SecretsFileFormat(FILE_FORMAT_JSON), valid: true, hasTemplate: false}, - {input: "json", output: SecretsFileFormat(FILE_FORMAT_JSON), valid: true, hasTemplate: false}, - {input: "DOTENV", output: SecretsFileFormat(FILE_FORMAT_DOTENV), valid: true, hasTemplate: false}, - {input: "dotenv", output: SecretsFileFormat(FILE_FORMAT_DOTENV), valid: true, hasTemplate: false}, - {input: "BASH", output: SecretsFileFormat(FILE_FORMAT_BASH), valid: true, hasTemplate: false}, - {input: "bash", output: SecretsFileFormat(FILE_FORMAT_BASH), valid: true, hasTemplate: false}, - {input: "PLAINTEXT", output: SecretsFileFormat(FILE_FORMAT_PLAINTEXT), valid: true, hasTemplate: false}, - {input: "plaintext", output: SecretsFileFormat(FILE_FORMAT_PLAINTEXT), valid: true, hasTemplate: false}, - {input: "invalid", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: false, hasTemplate: false}, - {input: "dotenv", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: true, hasTemplate: true}, - {input: "invalid", output: SecretsFileFormat(FILE_FORMAT_NONE), valid: false, hasTemplate: true}, - } - - secretsGroupTestCase := extractSecretsGroupsTestCase{ - description: "file type annotation", - contents: map[string]string{ - "conjur.org/conjur-secrets.cache": someValidSecrets, - "conjur.org/secret-file-format.cache": "", - "conjur.org/secret-file-path.cache": "this/relative/file", - }, - } - - resultSecretsGroups := SecretsGroups{ - "cache": { - Secrets: SecretsPaths{ - "test-password": "/test/password", - "test-username": "/test/username", - "url": "/test/url", - }, - FilePath: "/conjur/secrets/this/relative/file", - FilePerms: DEFAULT_FILE_PERMS, - FileTemplate: "", - }, - } - - for _, tc := range fileFormatTestCases { - secretsGroupTestCase.contents["conjur.org/secret-file-format.cache"] = tc.input - group := resultSecretsGroups["cache"] - group.FileFormat = tc.output - - if tc.hasTemplate { - secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = someValidTemplate - group.FileTemplate = someValidTemplate - } else { - secretsGroupTestCase.contents["conjur.org/secret-file-template.cache"] = "" - group.FileTemplate = "" - } - - if tc.valid { - secretsGroupTestCase.assert = assertExpectedSecretsGroups(resultSecretsGroups) - } else { - secretsGroupTestCase.assert = assertProperError("unknown file format") - } - - resultSecretsGroups["cache"] = group - - t.Run(secretsGroupTestCase.description, func(t *testing.T) { - secretGroups, err := ExtractSecretsGroupsFromAnnotations(secretsGroupTestCase.contents) - secretsGroupTestCase.assert(t, secretGroups, err) - }) - } -} From e5d8d492c73192149062a4fec430bc594cabe88d Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Tue, 28 Sep 2021 14:28:42 -0700 Subject: [PATCH 5/7] CR changes 2 --- pkg/secrets/pushtofile/secret_groups.go | 10 +--------- pkg/secrets/pushtofile/secret_spec.go | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/pkg/secrets/pushtofile/secret_groups.go b/pkg/secrets/pushtofile/secret_groups.go index 4d60be5d..fdf98c30 100644 --- a/pkg/secrets/pushtofile/secret_groups.go +++ b/pkg/secrets/pushtofile/secret_groups.go @@ -72,15 +72,7 @@ func NewSecretGroupsFromAnnotations(annotations map[string]string) (SecretGroups func parseFileFormat(fileFormat string) (string, error) { switch fileFormat { - case "yaml": - fallthrough - case "json": - fallthrough - case "dotenv": - fallthrough - case "bash": - fallthrough - case "plaintext": + case "yaml", "json", "dotenv", "bash", "plaintext": return fileFormat, nil case "": return "yaml", nil diff --git a/pkg/secrets/pushtofile/secret_spec.go b/pkg/secrets/pushtofile/secret_spec.go index d89a7193..642cfe20 100644 --- a/pkg/secrets/pushtofile/secret_spec.go +++ b/pkg/secrets/pushtofile/secret_spec.go @@ -33,21 +33,21 @@ func (t *SecretSpec) UnmarshalYAML(unmarshal func(v interface{}) error) error { // MAP var mapValue map[string]string err = unmarshal(&mapValue) - if err == nil { - if len(mapValue) != 1 { - return log.RecordedError(messages.CSPFK050E, "expected single key-value pair for secret specification") - } + if err != nil { + return log.RecordedError(messages.CSPFK050E, "unrecognized format for secret spec") + } - for k, v := range mapValue { - t.Id = v - t.Alias = k - } + if len(mapValue) != 1 { + return log.RecordedError(messages.CSPFK050E, "expected single key-value pair for secret specification") + } - return nil + for k, v := range mapValue { + t.Id = v + t.Alias = k } - // ELSE - return log.RecordedError(messages.CSPFK050E, "unrecognized format for secret spec") + return nil + } func NewSecretSpecs(raw []byte) ([]SecretSpec, error) { From c57ab06ec5c831b61f9d35884978ccaeb0bdefb9 Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Tue, 28 Sep 2021 15:24:24 -0700 Subject: [PATCH 6/7] CR changes 3 --- pkg/secrets/pushtofile/secret_groups_test.go | 103 ++++++------------- 1 file changed, 33 insertions(+), 70 deletions(-) diff --git a/pkg/secrets/pushtofile/secret_groups_test.go b/pkg/secrets/pushtofile/secret_groups_test.go index a0256f9d..32c16a2a 100644 --- a/pkg/secrets/pushtofile/secret_groups_test.go +++ b/pkg/secrets/pushtofile/secret_groups_test.go @@ -6,22 +6,10 @@ import ( "github.com/stretchr/testify/assert" ) -type assertFunc func(*testing.T, SecretGroups, error) - type extractSecretGroupsTestCase struct { - description string - contents map[string]string - assert assertFunc -} - -func assertExpectedSecretGroups(expected SecretGroups) assertFunc { - return func(t *testing.T, result SecretGroups, err error) { - if !assert.NoError(t, err) { - return - } - - assert.Equal(t, expected, result) - } + description string + input map[string]string + expectedOutput SecretGroups } const someValidSecrets = "- test/url\n- test-password: test/password\n- test-username: test/username\n" @@ -29,7 +17,7 @@ const someValidSecrets = "- test/url\n- test-password: test/password\n- test-use var extractSecretGroupsTestCases = []extractSecretGroupsTestCase{ { description: "valid annotations map", - contents: map[string]string{ + input: map[string]string{ "conjur.org/authn-identity": "host/conjur/authn-k8s/cluster/apps/inventory-api", "conjur.org/container-mode": "application", "conjur.org/secret-destination": "file", @@ -43,78 +31,53 @@ var extractSecretGroupsTestCases = []extractSecretGroupsTestCase{ "conjur.org/secret-file-template.cache": "some-template", "conjur.org/secret-file-format.cache": "yaml", }, - assert: assertExpectedSecretGroups( - SecretGroups{ - { - Label: "cache", - FilePath: "this/relative/path", - FileTemplate: "some-template", - ConjurSecretPathPrefix: "some/policy/path", - SecretSpecs: []SecretSpec{ - {Id: "test/url", Alias: "url"}, - {Id: "test/password", Alias: "test-password"}, - {Id: "test/username", Alias: "test-username"}, - }, - FileFormat: "yaml", - FilePerms: defaultFilePerms, + expectedOutput: SecretGroups{ + { + Label: "cache", + FilePath: "this/relative/path", + FileTemplate: "some-template", + ConjurSecretPathPrefix: "some/policy/path", + SecretSpecs: []SecretSpec{ + {Id: "test/url", Alias: "url"}, + {Id: "test/password", Alias: "test-password"}, + {Id: "test/username", Alias: "test-username"}, }, + FileFormat: "yaml", + FilePerms: defaultFilePerms, }, - ), - }, - { - description: "valid annotations map", - contents: map[string]string{ - "conjur.org/conjur-secrets.cache": someValidSecrets, - "conjur.org/secret-file-path.cache": "this/relative/path", - "conjur.org/conjur-secrets-policy-path.cache": "some/policy/path", - "conjur.org/secret-file-template.cache": "some-template", - "conjur.org/secret-file-format.cache": "yaml", }, - assert: assertExpectedSecretGroups( - SecretGroups{ - { - Label: "cache", - FilePath: "this/relative/path", - FileTemplate: "some-template", - ConjurSecretPathPrefix: "some/policy/path", - SecretSpecs: []SecretSpec{ - {Id: "test/url", Alias: "url"}, - {Id: "test/password", Alias: "test-password"}, - {Id: "test/username", Alias: "test-username"}, - }, - FileFormat: "yaml", - FilePerms: defaultFilePerms, - }, - }, - ), }, } func TestNewSecretGroupsFromAnnotations(t *testing.T) { for _, tc := range extractSecretGroupsTestCases { t.Run(tc.description, func(t *testing.T) { - secretGroups, err := NewSecretGroupsFromAnnotations(tc.contents) + resultSecretGroups, err := NewSecretGroupsFromAnnotations(tc.input) + + if !assert.NoError(t, err) { + return + } - tc.assert(t, secretGroups, err) + assert.Equal(t, tc.expectedOutput, resultSecretGroups) }) } } func TestExtractFileFormatFromAnnotations(t *testing.T) { type fileFormatTestCase struct { - input string - output string - valid bool + input string + expectedOutput string + valid bool } fileFormatTestCases := []fileFormatTestCase{ - {input: "", output: "yaml", valid: true}, - {input: "yaml", output: "yaml", valid: true}, - {input: "json", output: "json", valid: true}, - {input: "dotenv", output: "dotenv", valid: true}, - {input: "bash", output: "bash", valid: true}, - {input: "plaintext", output: "plaintext", valid: true}, - {input: "invalid", output: "", valid: false}, + {input: "", expectedOutput: "yaml", valid: true}, + {input: "yaml", expectedOutput: "yaml", valid: true}, + {input: "json", expectedOutput: "json", valid: true}, + {input: "dotenv", expectedOutput: "dotenv", valid: true}, + {input: "bash", expectedOutput: "bash", valid: true}, + {input: "plaintext", expectedOutput: "plaintext", valid: true}, + {input: "invalid", expectedOutput: "", valid: false}, } someValidAnnotationsMap := map[string]string{ @@ -130,7 +93,7 @@ func TestExtractFileFormatFromAnnotations(t *testing.T) { if tc.valid { assert.NoError(t, err) - assert.Equal(t, tc.output, secretGroups[0].FileFormat) + assert.Equal(t, tc.expectedOutput, secretGroups[0].FileFormat) } else { assert.Nil(t, secretGroups) assert.Contains(t, err.Error(), "Unknown file format") From 9b561be015521f1271a2c17ea2de8e39dc49a2d7 Mon Sep 17 00:00:00 2001 From: Samir Shetty Date: Wed, 29 Sep 2021 07:57:10 -0700 Subject: [PATCH 7/7] Minor test cleanup --- pkg/secrets/pushtofile/secret_groups_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/secrets/pushtofile/secret_groups_test.go b/pkg/secrets/pushtofile/secret_groups_test.go index 32c16a2a..64bd1ca4 100644 --- a/pkg/secrets/pushtofile/secret_groups_test.go +++ b/pkg/secrets/pushtofile/secret_groups_test.go @@ -54,10 +54,7 @@ func TestNewSecretGroupsFromAnnotations(t *testing.T) { t.Run(tc.description, func(t *testing.T) { resultSecretGroups, err := NewSecretGroupsFromAnnotations(tc.input) - if !assert.NoError(t, err) { - return - } - + assert.NoError(t, err) assert.Equal(t, tc.expectedOutput, resultSecretGroups) }) }