From b70f3aefe528b27344a717c99c3fdbb02d5c8f4d Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 28 Jul 2020 16:55:50 -0700 Subject: [PATCH 01/17] First draft of LoginDiscoveryConfig CRD --- deploy/crd.yaml | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 deploy/crd.yaml diff --git a/deploy/crd.yaml b/deploy/crd.yaml new file mode 100644 index 00000000..863f423d --- /dev/null +++ b/deploy/crd.yaml @@ -0,0 +1,44 @@ +#@ load("@ytt:data", "data") + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: logindiscoveryconfigs.suzerain-io.github.io +spec: + group: suzerain-io.github.io + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + required: [server, certificateAuthorityData] + properties: + server: + type: string + minLength: 1 + certificateAuthorityData: + type: string + minLength: 1 + identityProviders: + type: array + items: + type: object + properties: + name: + type: string + idpType: + type: string + pattern: '^token$' #! validation via regexp pattern match + scope: Namespaced + names: + plural: logindiscoveryconfigs + singular: logindiscoveryconfig + kind: LoginDiscoveryConfig + shortNames: + - ldc From 7ba43e0c3fb49bc984042fce2c4e71ae542d667d Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 29 Jul 2020 13:14:50 -0700 Subject: [PATCH 02/17] More validations on the LoginDiscoveryConfig CRD Signed-off-by: Ryan Richard --- deploy/crd.yaml | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/deploy/crd.yaml b/deploy/crd.yaml index 863f423d..a7b884cd 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -1,5 +1,21 @@ #@ load("@ytt:data", "data") +#! Example of valid LoginDiscoveryConfig object: +#! --- +#! apiVersion: suzerain-io.github.io/v1alpha1 +#! kind: LoginDiscoveryConfig +#! metadata: +#! name: login-discovery +#! namespace: integration +#! spec: +#! server: https://foo +#! certificateAuthorityData: bar +#! identityProviders: +#! - name: baz +#! type: token +#! - name: bat +#! type: token + --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition @@ -8,20 +24,24 @@ metadata: spec: group: suzerain-io.github.io versions: + #! Any changes to these schemas should also be reflected in the types.go file(s) + #! in https://github.com/suzerain-io/placeholder-name-api/tree/main/pkg/apis/placeholder - name: v1alpha1 served: true storage: true schema: openAPIV3Schema: type: object + required: [spec] properties: spec: type: object - required: [server, certificateAuthorityData] + required: [server, certificateAuthorityData, identityProviders] properties: server: type: string minLength: 1 + pattern: '^https://' certificateAuthorityData: type: string minLength: 1 @@ -29,12 +49,15 @@ spec: type: array items: type: object + required: [name, type] properties: name: type: string - idpType: + minLength: 1 + pattern: '^[a-zA-Z0-9]+(?:(-|_)?[a-zA-Z0-9]+)+$' + type: #! The name of this property is "type" (not declaring the type of a property). type: string - pattern: '^token$' #! validation via regexp pattern match + pattern: '^token$' scope: Namespaced names: plural: logindiscoveryconfigs From a8f3c62d37d9c72cc4a5d84c8bea1496c1762b6a Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 29 Jul 2020 13:17:55 -0700 Subject: [PATCH 03/17] Remove identity provider list from LoginDiscoveryConfig CRD Because we're not going to need it for the current story Signed-off-by: Andrew Keesler --- deploy/crd.yaml | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/deploy/crd.yaml b/deploy/crd.yaml index a7b884cd..de814775 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -10,11 +10,6 @@ #! spec: #! server: https://foo #! certificateAuthorityData: bar -#! identityProviders: -#! - name: baz -#! type: token -#! - name: bat -#! type: token --- apiVersion: apiextensions.k8s.io/v1 @@ -36,7 +31,7 @@ spec: properties: spec: type: object - required: [server, certificateAuthorityData, identityProviders] + required: [server, certificateAuthorityData] properties: server: type: string @@ -45,19 +40,6 @@ spec: certificateAuthorityData: type: string minLength: 1 - identityProviders: - type: array - items: - type: object - required: [name, type] - properties: - name: - type: string - minLength: 1 - pattern: '^[a-zA-Z0-9]+(?:(-|_)?[a-zA-Z0-9]+)+$' - type: #! The name of this property is "type" (not declaring the type of a property). - type: string - pattern: '^token$' scope: Namespaced names: plural: logindiscoveryconfigs From a5f7de429d07acf0ca2ca419fa7251336891e84e Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 29 Jul 2020 17:22:25 -0700 Subject: [PATCH 04/17] First commit of PublisherController - Also upgrade go-client and api dependencies, and add controller-go as a dependency Signed-off-by: Ryan Richard --- go.mod | 10 +- go.sum | 50 ++++--- .../controller/logindiscovery/publisher.go | 77 +++++++++++ .../logindiscovery/publisher_test.go | 129 ++++++++++++++++++ 4 files changed, 239 insertions(+), 27 deletions(-) create mode 100644 internal/controller/logindiscovery/publisher.go create mode 100644 internal/controller/logindiscovery/publisher_test.go diff --git a/go.mod b/go.mod index 7764b378..fd0a02e9 100644 --- a/go.mod +++ b/go.mod @@ -5,14 +5,14 @@ go 1.14 require ( github.com/davecgh/go-spew v1.1.1 github.com/golang/mock v1.4.3 - github.com/golangci/golangci-lint v1.28.1 - github.com/google/go-cmp v0.4.0 + github.com/golangci/golangci-lint v1.29.0 + github.com/google/go-cmp v0.5.0 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 - github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e - github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95 - golang.org/x/tools v0.0.0-20200707134715-9e0a013e855f // indirect + github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 + github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 + github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c k8s.io/api v0.19.0-rc.0 k8s.io/apimachinery v0.19.0-rc.0 k8s.io/apiserver v0.19.0-rc.0 diff --git a/go.sum b/go.sum index 503ef603..1d7c7d9b 100644 --- a/go.sum +++ b/go.sum @@ -44,8 +44,10 @@ github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= +github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 h1:JYp7IbQjafoB+tBA3gMyHYHrpOtNuDiK/uB5uXxq5wM= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4 h1:Hs82Z41s6SdL1CELW+XaDYmOH4hkBN4/N9og/AsOv7E= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= @@ -218,8 +220,8 @@ github.com/golangci/gocyclo v0.0.0-20180528144436-0a533e8fa43d h1:pXTK/gkVNs7Zyy github.com/golangci/gocyclo v0.0.0-20180528144436-0a533e8fa43d/go.mod h1:ozx7R9SIwqmqf5pRP90DhR2Oay2UIjGuKheCBCNwAYU= github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a h1:iR3fYXUjHCR97qWS8ch1y9zPNsgXThGwjKPrYfqMPks= github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU= -github.com/golangci/golangci-lint v1.28.1 h1:yU1ejxsNdqBwSKVFqbBseJfSAhCp7mXwp5CP7+cwCaE= -github.com/golangci/golangci-lint v1.28.1/go.mod h1:ImsQ2aOofgQ4FNsGZxD5DzRZNxhVXux3AHpjfM6kSQo= +github.com/golangci/golangci-lint v1.29.0 h1:0ufaO3l2R1R712cFC+KT3TtwO/IOcsloKZBavRtzrBk= +github.com/golangci/golangci-lint v1.29.0/go.mod h1:Iq2GFBB9OoolSDWD81m0iJ2MR4MwDVbi4eC93fO7wh0= github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc h1:gLLhTLMk2/SutryVJ6D4VZCU3CUqr8YloG7FPIBWFpI= github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc/go.mod h1:e5tpTHCfVze+7EpLEozzMB3eafxo2KT5veNg1k6byQU= github.com/golangci/lint-1 v0.0.0-20191013205115-297bf364a8e0 h1:MfyDlzVjl1hoaPzPD4Gpb/QgoRfSBR0jdhwGyAWwMSA= @@ -242,6 +244,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w= +github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -340,8 +344,8 @@ github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/kyoh86/exportloopref v0.1.4 h1:t8QP+vBUykOFp6Bks/ZVYm3+Rp3+aj+AKWpGXgK4anA= -github.com/kyoh86/exportloopref v0.1.4/go.mod h1:h1rDl2Kdj97+Kwh4gdz3ujE7XHmH51Q0lUiZ1z4NLj8= +github.com/kyoh86/exportloopref v0.1.7 h1:u+iHuTbkbTS2D/JP7fCuZDo/t3rBVGo3Hf58Rc+lQVY= +github.com/kyoh86/exportloopref v0.1.7/go.mod h1:h1rDl2Kdj97+Kwh4gdz3ujE7XHmH51Q0lUiZ1z4NLj8= github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= @@ -358,8 +362,8 @@ github.com/matoous/godox v0.0.0-20190911065817-5d6d842e92eb h1:RHba4YImhrUVQDHUC github.com/matoous/godox v0.0.0-20190911065817-5d6d842e92eb/go.mod h1:1BELzlh859Sh1c6+90blK8lbYy0kwQf1bYlBhBysy1s= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= -github.com/mattn/go-colorable v0.1.6 h1:6Su7aK7lXmJ/U79bYtBjLNaha4Fs1Rg9plHpcH+vvnE= -github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= +github.com/mattn/go-colorable v0.1.7 h1:bQGKb3vps/j0E9GfJQ03JyhRuxsvdAanXlT9BTw3mdw= +github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= @@ -403,8 +407,8 @@ github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d h1:AREM5mwr4u1 github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d/go.mod h1:o96djdrsSGy3AWPyBgZMAGfxZNfgntdJG+11KU4QvbU= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= -github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132 h1:NjznefjSrral0MiR4KlB41io/d3OklvhcgQUdfZTqJE= -github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132/go.mod h1:wBEpHwM2OdmeNpdCvRPUlkEbBuaFmcK4Wv8Q7FuGW3c= +github.com/nishanths/exhaustive v0.0.0-20200708172631-8866003e3856 h1:W3KBC2LFyfgd+wNudlfgCCsTo4q97MeNWrfz8/wSdSc= +github.com/nishanths/exhaustive v0.0.0-20200708172631-8866003e3856/go.mod h1:wBEpHwM2OdmeNpdCvRPUlkEbBuaFmcK4Wv8Q7FuGW3c= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= @@ -460,7 +464,7 @@ github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95 h1:L8QM9bvf github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95/go.mod h1:rlzQ04UMyJXu/aOvhd8qT+hvDrFpiwqp8MRXDY9szc0= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= -github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= +github.com/rogpeppe/go-internal v1.6.0/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryancurrah/gomodguard v1.1.0 h1:DWbye9KyMgytn8uYpuHkwf0RHqAYO6Ay/D0TbCpPtVU= github.com/ryancurrah/gomodguard v1.1.0/go.mod h1:4O8tr7hBODaGE6VIhfJDHcwzh5GUccKSJBU0UMXJFVM= @@ -472,6 +476,8 @@ github.com/sclevine/spec v1.4.0/go.mod h1:LvpgJaFyvQzRvc1kaDs0bulYwzC70PbiYjC4Qn github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= github.com/securego/gosec/v2 v2.3.0 h1:y/9mCF2WPDbSDpL3QDWZD3HHGrSYw0QSHnCqTfs4JPE= github.com/securego/gosec/v2 v2.3.0/go.mod h1:UzeVyUXbxukhLeHKV3VVqo7HdoQR9MrRfFmZYotn8ME= +github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c h1:W65qqJCIOVP4jpqPQ0YvHYKwcMEMVWIzWC5iNQQfBTU= +github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c/go.mod h1:/PevMnwAxekIXwN8qQyfc5gl2NlkB3CQlkizAbOkeBs= github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada/go.mod h1:WWnYX4lzhCH5h/3YBfyVA3VbLYjlMZZAQcW9ojMexNc= github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4/go.mod h1:qsXQc7+bwAM3Q1u/4XEfrquwF8Lw7D7y5cD8CuHnfIc= github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e h1:MZM7FHLqUHYI0Y/mQAt3d2aYa0SiNms/hFqC9qJYolM= @@ -524,12 +530,12 @@ github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200714184318-8ad91581433a h1:ycG6TufZM7ZDrgBklkXEMauvSyh44JQDvSUdJawAjGA= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200714184318-8ad91581433a/go.mod h1:bNHheAnmAISdW/ZYTnhCmg8QQKwA5WD64ZvPdsTrWjw= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e h1:PavRTcjOk0SUtIRUY4OaFXB3CVYFLTIlxLptUawLB1o= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e/go.mod h1:bNHheAnmAISdW/ZYTnhCmg8QQKwA5WD64ZvPdsTrWjw= -github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95 h1:O1aoszdMJEekLXD7pNWP23vq0g8eXLp40AQgY8Hj+Sw= -github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95/go.mod h1:NwirXEgd1VaA+6R0W7PYL/ae6wfPT3vA+tu7POAArjU= +github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 h1:2NQMYv6IIjcYGkNdezwcY7/MS/3goEZThlz7u6ml9j4= +github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 h1:xnco3XJMrvlwyQJfKoyVPciATvCJ3Y6SY2D8gI2DT2E= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= +github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c h1:UQVAfjF71g+v2L0ZT8MedKxrdIpx4KrHqvefnmW1AoU= +github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c/go.mod h1:J/D+4tWlKpLpTd8Jotb7FnLl1OeT2KrUOP3Wc3ooqis= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2 h1:Xr9gkxfOP0KQWXKNqmwe8vEeSUiUj4Rlee9CMVX2ZUQ= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM= github.com/tetafro/godot v0.4.2 h1:Dib7un+rYJFUi8vN0Bk6EHheKy6fv6ZzFURHw75g6m8= @@ -551,7 +557,7 @@ github.com/uudashr/gocognit v1.0.1 h1:MoG2fZ0b/Eo7NXoIwCVFLG5JED3qgQz5/NEE+rOsjP github.com/uudashr/gocognit v1.0.1/go.mod h1:j44Ayx2KW4+oB6SWMv8KsmHzZrOInQav7D3cQMJ5JUM= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasthttp v1.12.0/go.mod h1:229t1eWu9UXTPmoUkbpN/fctKPBY4IJoFXQnxHGXy6E= -github.com/valyala/quicktemplate v1.5.0/go.mod h1:v7yYWpBEiutDyNfVaph6oC/yKwejzVyTX/2cwwHxyok= +github.com/valyala/quicktemplate v1.5.1/go.mod h1:v7yYWpBEiutDyNfVaph6oC/yKwejzVyTX/2cwwHxyok= github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a/go.mod h1:v3UYOV9WzVtRmSR+PDvWpU/qWl4Wa5LApYYX4ZtKbio= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= @@ -734,13 +740,12 @@ golang.org/x/tools v0.0.0-20200324003944-a576cf524670/go.mod h1:Sl4aGygMT6LrqrWc golang.org/x/tools v0.0.0-20200331202046-9d5940d49312/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200414032229-332987a829c3/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200428185508-e9a00ec82136/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200602230032-c00d67ef29d0/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200625211823-6506e20df31f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200707134715-9e0a013e855f h1:y1MEz+/UGBJqz2A4K9QOu4TeXQ9Vs5MlmvhETgaR0Kg= -golang.org/x/tools v0.0.0-20200707134715-9e0a013e855f/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= +golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= +golang.org/x/tools v0.0.0-20200710042808-f1c4188a97a1 h1:rD1FcWVsRaMY+l8biE9jbWP5MS/CJJ/90a9TMkMgNrM= +golang.org/x/tools v0.0.0-20200710042808-f1c4188a97a1/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= 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= @@ -787,6 +792,7 @@ google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.24.0 h1:UhZDfRO8JRQru4/+LlLE0BRKGF8L+PICnvYZmx/fEGA= google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= +gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -850,8 +856,8 @@ k8s.io/kube-openapi v0.0.0-20200615155156-dffdd1682719 h1:n/ElZyI1dzFPXKS8nZMw8w k8s.io/kube-openapi v0.0.0-20200615155156-dffdd1682719/go.mod h1:bfCVj+qXcEaE5SCvzBaqpOySr6tuCcpPKqF6HD8nyCw= k8s.io/utils v0.0.0-20200619165400-6e3d28b6ed19 h1:7Nu2dTj82c6IaWvL7hImJzcXoTPz1MsSCH7r+0m6rfo= k8s.io/utils v0.0.0-20200619165400-6e3d28b6ed19/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= -mvdan.cc/gofumpt v0.0.0-20200513141252-abc0db2c416a h1:TTEzidAa7rn93JGy1ACigx6o9VcsRLKG7qICdErmvUs= -mvdan.cc/gofumpt v0.0.0-20200513141252-abc0db2c416a/go.mod h1:4q/PlrZKQLU5MowSvCKM3U4xJUPtJ8vKWx7vsWFJ3MI= +mvdan.cc/gofumpt v0.0.0-20200709182408-4fd085cb6d5f h1:gi7cb8HTDZ6q8VqsUpkdoFi3vxwHMneQ6+Q5Ap5hjPE= +mvdan.cc/gofumpt v0.0.0-20200709182408-4fd085cb6d5f/go.mod h1:9VQ397fNXEnF84t90W4r4TRCQK+pg9f8ugVfyj+S26w= mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed h1:WX1yoOaKQfddO/mLzdV4wptyWgoH/6hwLs7QHTixo0I= mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed/go.mod h1:Xkxe497xwlCKkIaQYRfC7CSLworTXY9RMqwhhCm+8Nc= mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b h1:DxJ5nJdkhDlLok9K6qO+5290kphDJbHOQO1DFFFTeBo= diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go new file mode 100644 index 00000000..1d5d32d0 --- /dev/null +++ b/internal/controller/logindiscovery/publisher.go @@ -0,0 +1,77 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package logindiscovery + +import ( + "encoding/base64" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/clientcmd" + + "github.com/suzerain-io/controller-go" + placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + placeholderclientset "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned" +) + +const ( + clusterInfoName = "cluster-info" + clusterInfoNamespace = "kube-public" + clusterInfoConfigMapKey = "kubeconfig" + + configName = "placeholder-name-config" +) + +type publisherController struct { + namespace string + kubeClient kubernetes.Interface + placeholderClient placeholderclientset.Interface +} + +func NewPublisherController(namespace string, kubeClient kubernetes.Interface, placeholderClient placeholderclientset.Interface) controller.Controller { + return controller.New( + controller.Config{ + Name: "publisher-controller", + Syncer: &publisherController{ + namespace: namespace, + kubeClient: kubeClient, + placeholderClient: placeholderClient, + }, + }, + ) +} + +func (c *publisherController) Sync(ctx controller.Context) error { + configMap, _ := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) + kubeConfig := configMap.Data[clusterInfoConfigMapKey] // TODO also handle when the key is not found + + config, _ := clientcmd.Load([]byte(kubeConfig)) + + var certificateAuthorityData, server string + for _, v := range config.Clusters { + certificateAuthorityData = base64.StdEncoding.EncodeToString(v.CertificateAuthorityData) + server = v.Server + break + } + + discoveryConfig := placeholderv1alpha1.LoginDiscoveryConfig{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: configName, + Namespace: c.namespace, + }, + Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Server: server, + CertificateAuthorityData: certificateAuthorityData, + }, + } + _, _ = c.placeholderClient. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(c.namespace). + Create(ctx.Context, &discoveryConfig, metav1.CreateOptions{}) + + return nil +} diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go new file mode 100644 index 00000000..b0bae88a --- /dev/null +++ b/internal/controller/logindiscovery/publisher_test.go @@ -0,0 +1,129 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package logindiscovery + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + kubernetesfake "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" + + "github.com/suzerain-io/controller-go" + placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + placeholderfake "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned/fake" +) + +// TODO: add test for ConfigMap does NOT exist +// TODO: add test for when LoginDiscoveryConfig already exists +// There should be 4 tests of the above cross product + +// TODO test when the expected key in the configmap does not exist + +func TestRun(t *testing.T) { + spec.Run(t, "publisher", func(t *testing.T, when spec.G, it spec.S) { + const installedInNamespace = "some-namespace" + + var r *require.Assertions + + var subject controller.Controller + var kubeClient *kubernetesfake.Clientset + var placeholderClient *placeholderfake.Clientset + var timeoutContext context.Context + var timeoutContextCancel context.CancelFunc + var controllerContext *controller.Context + + var expectedLoginDiscoveryConfig = func(expectedNamespace, expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *placeholderv1alpha1.LoginDiscoveryConfig) { + expectedLoginDiscoveryConfigGVR := schema.GroupVersionResource{ + Group: placeholderv1alpha1.GroupName, + Version: "v1alpha1", + Resource: "logindiscoveryconfigs", + } + expectedLoginDiscoveryConfig := &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "placeholder-name-config", + Namespace: expectedNamespace, + }, + Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Server: expectedServerURL, + CertificateAuthorityData: expectedCAData, + }, + } + return expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig + } + + it.Before(func() { + r = require.New(t) + kubeClient = kubernetesfake.NewSimpleClientset() + placeholderClient = placeholderfake.NewSimpleClientset() + timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) + subject = NewPublisherController(installedInNamespace, kubeClient, placeholderClient) + controllerContext = &controller.Context{ + Context: timeoutContext, + Name: subject.Name(), + Key: controller.Key{ + Namespace: "kube-public", + Name: "cluster-info", + }, + } + }) + + when("when there is a cluster-info ConfigMap in the kube-public namespace", func() { + const caData = "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=" // "some-certificate-authority-data" base64 encoded + const kubeServerURL = "https://some-server" + + it.Before(func() { + clusterInfo := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-info", + Namespace: "kube-public", + }, + // Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below. + Data: map[string]string{ + "kubeconfig": strings.ReplaceAll(` + kind: Config + apiVersion: v1 + clusters: + - name: "" + cluster: + certificate-authority-data: "`+caData+`" + server: "`+kubeServerURL+`"`, "\t", " "), + "uninteresting-key": "uninteresting-value", + }, + } + err := kubeClient.Tracker().Add(clusterInfo) + r.NoError(err) + }) + + when("when the LoginDiscoveryConfig does not already exist", func() { + it("creates a LoginDiscoveryConfig", func() { + defer timeoutContextCancel() + + err := controller.TestSync(t, subject, *controllerContext) // Call the controller's sync method once + r.NoError(err) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData) + expectedActions := []coretesting.Action{ + coretesting.NewCreateAction(expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig), + } + + // Expect a LoginDiscoveryConfig to be created with the fields from the cluster-info ConfigMap + actualCreatedObject := placeholderClient.Actions()[0].(coretesting.CreateActionImpl).Object + r.Equal(expectedLoginDiscoveryConfig, actualCreatedObject) + r.Equal(expectedActions, placeholderClient.Actions()) + }) + }, spec.Parallel()) + }, spec.Parallel()) + }, spec.Report(report.Terminal{})) +} From e0cac97084734384ee40e4962f996dd5943668ef Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 29 Jul 2020 18:18:42 -0700 Subject: [PATCH 05/17] More tests for the PublisherController - Also, don't repeat `spec.Parallel()` because, according to the docs for the spec package, "options are inherited by subgroups and subspecs" - Two tests are left pending to be filled in on the next commit --- cmd/placeholder-name/main_test.go | 12 +- .../controller/logindiscovery/publisher.go | 10 +- .../logindiscovery/publisher_test.go | 132 +++++++++++++----- 3 files changed, 108 insertions(+), 46 deletions(-) diff --git a/cmd/placeholder-name/main_test.go b/cmd/placeholder-name/main_test.go index 37dd5b0c..4c45384e 100644 --- a/cmd/placeholder-name/main_test.go +++ b/cmd/placeholder-name/main_test.go @@ -64,7 +64,7 @@ func TestRun(t *testing.T) { err := run(envGetter, tokenExchanger, buffer, 30*time.Second) r.EqualError(err, "failed to login: environment variable not set: PLACEHOLDER_NAME_K8S_API_ENDPOINT") }) - }, spec.Parallel()) + }) when("the token exchange fails", func() { it.Before(func() { @@ -77,7 +77,7 @@ func TestRun(t *testing.T) { err := run(envGetter, tokenExchanger, buffer, 30*time.Second) r.EqualError(err, "failed to login: some error") }) - }, spec.Parallel()) + }) when("the JSON encoder fails", func() { it.Before(func() { @@ -92,7 +92,7 @@ func TestRun(t *testing.T) { err := run(envGetter, tokenExchanger, &library.ErrorWriter{ReturnError: fmt.Errorf("some IO error")}, 30*time.Second) r.EqualError(err, "failed to marshal response to stdout: some IO error") }) - }, spec.Parallel()) + }) when("the token exchange times out", func() { it.Before(func() { @@ -112,7 +112,7 @@ func TestRun(t *testing.T) { err := run(envGetter, tokenExchanger, buffer, 1*time.Millisecond) r.EqualError(err, "failed to login: context deadline exceeded") }) - }, spec.Parallel()) + }) when("the token exchange succeeds", func() { var actualToken, actualCaBundle, actualAPIEndpoint string @@ -146,6 +146,6 @@ func TestRun(t *testing.T) { }` r.JSONEq(expected, buffer.String()) }) - }, spec.Parallel()) - }, spec.Report(report.Terminal{})) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) } diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 1d5d32d0..2795dda5 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -45,8 +45,14 @@ func NewPublisherController(namespace string, kubeClient kubernetes.Interface, p } func (c *publisherController) Sync(ctx controller.Context) error { - configMap, _ := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) - kubeConfig := configMap.Data[clusterInfoConfigMapKey] // TODO also handle when the key is not found + configMap, err := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) + if err != nil { + return nil // TODO should this return an error? and should it log? + } + kubeConfig, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey] + if !kubeConfigPresent { + return nil // TODO should this return an error? and should it log? + } config, _ := clientcmd.Load([]byte(kubeConfig)) diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index b0bae88a..72dffbd7 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -25,12 +25,6 @@ import ( placeholderfake "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned/fake" ) -// TODO: add test for ConfigMap does NOT exist -// TODO: add test for when LoginDiscoveryConfig already exists -// There should be 4 tests of the above cross product - -// TODO test when the expected key in the configmap does not exist - func TestRun(t *testing.T) { spec.Run(t, "publisher", func(t *testing.T, when spec.G, it spec.S) { const installedInNamespace = "some-namespace" @@ -79,19 +73,18 @@ func TestRun(t *testing.T) { } }) - when("when there is a cluster-info ConfigMap in the kube-public namespace", func() { + when("there is a cluster-info ConfigMap in the kube-public namespace", func() { const caData = "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=" // "some-certificate-authority-data" base64 encoded const kubeServerURL = "https://some-server" + var clusterInfoConfigMap *corev1.ConfigMap - it.Before(func() { - clusterInfo := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster-info", - Namespace: "kube-public", - }, - // Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below. - Data: map[string]string{ - "kubeconfig": strings.ReplaceAll(` + when("the ConfigMap has the expected `kubeconfig` top-level data key", func() { + it.Before(func() { + clusterInfoConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, + // Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below. + Data: map[string]string{ + "kubeconfig": strings.ReplaceAll(` kind: Config apiVersion: v1 clusters: @@ -99,31 +92,94 @@ func TestRun(t *testing.T) { cluster: certificate-authority-data: "`+caData+`" server: "`+kubeServerURL+`"`, "\t", " "), - "uninteresting-key": "uninteresting-value", + "uninteresting-key": "uninteresting-value", + }, + } + err := kubeClient.Tracker().Add(clusterInfoConfigMap) + r.NoError(err) + }) + + when("the LoginDiscoveryConfig does not already exist", func() { + it("creates a LoginDiscoveryConfig", func() { + defer timeoutContextCancel() + + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData) + expectedActions := []coretesting.Action{ + coretesting.NewCreateAction(expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig), + } + + // Expect a LoginDiscoveryConfig to be created with the fields from the cluster-info ConfigMap + actualCreatedObject := placeholderClient.Actions()[0].(coretesting.CreateActionImpl).Object + r.Equal(expectedLoginDiscoveryConfig, actualCreatedObject) + r.Equal(expectedActions, placeholderClient.Actions()) + }) + }) + + when("the LoginDiscoveryConfig already exists", func() { + when("the LoginDiscoveryConfig is already up to date according to the data in the ConfigMap", func() { + it.Before(func() { + // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data matches the ConfigMap's data + }) + + it.Pend("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + r.Empty(placeholderClient.Actions()) + }) + }) + + when("the LoginDiscoveryConfig is stale compared to the data in the ConfigMap", func() { + it.Before(func() { + // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data does not match the ConfigMap's data + }) + + it.Pend("updates the existing LoginDiscoveryConfig", func() { + // TODO + }) + }) + }) + }) + + when("the ConfigMap is missing the expected `kubeconfig` top-level data key", func() { + it.Before(func() { + clusterInfoConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, + Data: map[string]string{ + "these are not the droids you're looking for": "uninteresting-value", + }, + } + err := kubeClient.Tracker().Add(clusterInfoConfigMap) + r.NoError(err) + }) + + it("keeps waiting for it to exist", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + r.Empty(placeholderClient.Actions()) + }) + }) + }) + + when("there is not a cluster-info ConfigMap in the kube-public namespace", func() { + it.Before(func() { + unrelatedConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oops this is not the cluster-info ConfigMap", + Namespace: "kube-public", }, } - err := kubeClient.Tracker().Add(clusterInfo) + err := kubeClient.Tracker().Add(unrelatedConfigMap) r.NoError(err) }) - when("when the LoginDiscoveryConfig does not already exist", func() { - it("creates a LoginDiscoveryConfig", func() { - defer timeoutContextCancel() - - err := controller.TestSync(t, subject, *controllerContext) // Call the controller's sync method once - r.NoError(err) - - expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData) - expectedActions := []coretesting.Action{ - coretesting.NewCreateAction(expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig), - } - - // Expect a LoginDiscoveryConfig to be created with the fields from the cluster-info ConfigMap - actualCreatedObject := placeholderClient.Actions()[0].(coretesting.CreateActionImpl).Object - r.Equal(expectedLoginDiscoveryConfig, actualCreatedObject) - r.Equal(expectedActions, placeholderClient.Actions()) - }) - }, spec.Parallel()) - }, spec.Parallel()) - }, spec.Report(report.Terminal{})) + it("keeps waiting for one", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + r.Empty(placeholderClient.Actions()) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) } From 9a859875a7ac4107f1b48e4f1bd928242bb7d87f Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 30 Jul 2020 10:39:15 -0400 Subject: [PATCH 06/17] logindiscovery: add tests for conditional update and error cases - Also add some log lines for better observability of behavior. Signed-off-by: Andrew Keesler --- go.mod | 2 +- go.sum | 2 + .../controller/logindiscovery/publisher.go | 81 +++++++++- .../logindiscovery/publisher_test.go | 149 ++++++++++++++++-- 4 files changed, 213 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index fd0a02e9..6786bf68 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 - github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 + github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c k8s.io/api v0.19.0-rc.0 k8s.io/apimachinery v0.19.0-rc.0 diff --git a/go.sum b/go.sum index 1d7c7d9b..3a317f7a 100644 --- a/go.sum +++ b/go.sum @@ -534,6 +534,8 @@ github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 h1:2NQMY github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 h1:xnco3XJMrvlwyQJfKoyVPciATvCJ3Y6SY2D8gI2DT2E= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b h1:7Fuizf0c3ffqyHj7X4AvXnYNFJHbSgHKjuDxDsxeQ8A= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c h1:UQVAfjF71g+v2L0ZT8MedKxrdIpx4KrHqvefnmW1AoU= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c/go.mod h1:J/D+4tWlKpLpTd8Jotb7FnLl1OeT2KrUOP3Wc3ooqis= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2 h1:Xr9gkxfOP0KQWXKNqmwe8vEeSUiUj4Rlee9CMVX2ZUQ= diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 2795dda5..adbeb3cc 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -6,11 +6,15 @@ SPDX-License-Identifier: Apache-2.0 package logindiscovery import ( + "context" "encoding/base64" + "fmt" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" + "k8s.io/klog/v2" "github.com/suzerain-io/controller-go" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" @@ -45,13 +49,27 @@ func NewPublisherController(namespace string, kubeClient kubernetes.Interface, p } func (c *publisherController) Sync(ctx controller.Context) error { - configMap, err := c.kubeClient.CoreV1().ConfigMaps(clusterInfoNamespace).Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) - if err != nil { - return nil // TODO should this return an error? and should it log? + configMap, err := c.kubeClient. + CoreV1(). + ConfigMaps(clusterInfoNamespace). + Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("failed to get %s configmap: %w", clusterInfoName, err) } + if notFound { + klog.InfoS( + "could not find config map", + "configmap", + klog.KRef(clusterInfoNamespace, clusterInfoName), + ) + return nil + } + kubeConfig, kubeConfigPresent := configMap.Data[clusterInfoConfigMapKey] if !kubeConfigPresent { - return nil // TODO should this return an error? and should it log? + klog.InfoS("could not find kubeconfig configmap key") + return nil } config, _ := clientcmd.Load([]byte(kubeConfig)) @@ -74,10 +92,57 @@ func (c *publisherController) Sync(ctx controller.Context) error { CertificateAuthorityData: certificateAuthorityData, }, } - _, _ = c.placeholderClient. - PlaceholderV1alpha1(). - LoginDiscoveryConfigs(c.namespace). - Create(ctx.Context, &discoveryConfig, metav1.CreateOptions{}) + if err := c.createOrUpdateLoginDiscoveryConfig(ctx.Context, &discoveryConfig); err != nil { + return err + } return nil } + +func (c *publisherController) createOrUpdateLoginDiscoveryConfig( + ctx context.Context, + discoveryConfig *placeholderv1alpha1.LoginDiscoveryConfig, +) error { + loginDiscoveryConfigs := c.placeholderClient. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(c.namespace) + + existingDiscoveryConfig, err := loginDiscoveryConfigs.Get( + ctx, + discoveryConfig.Name, + metav1.GetOptions{}, + ) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("could not get logindiscoveryconfig: %w", err) + } + + if notFound { + if _, err := loginDiscoveryConfigs.Create( + ctx, + discoveryConfig, + metav1.CreateOptions{}, + ); err != nil { + return fmt.Errorf("could not create logindiscoveryconfig: %w", err) + } + } else if !equal(existingDiscoveryConfig, discoveryConfig) { + // Update just the fields we care about. + existingDiscoveryConfig.Spec.Server = discoveryConfig.Spec.Server + existingDiscoveryConfig.Spec.CertificateAuthorityData = discoveryConfig.Spec.CertificateAuthorityData + + if _, err := loginDiscoveryConfigs.Update( + ctx, + existingDiscoveryConfig, + metav1.UpdateOptions{}, + ); err != nil { + return fmt.Errorf("could not update logindiscoveryconfig: %w", err) + } + } + + return nil +} + +func equal(a, b *placeholderv1alpha1.LoginDiscoveryConfig) bool { + return a.Spec.Server == b.Spec.Server && + a.Spec.CertificateAuthorityData == b.Spec.CertificateAuthorityData +} diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index 72dffbd7..eff0dacb 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -7,6 +7,7 @@ package logindiscovery import ( "context" + "errors" "strings" "testing" "time" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" @@ -106,38 +108,144 @@ func TestRun(t *testing.T) { err := controller.TestSync(t, subject, *controllerContext) r.NoError(err) - expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig(installedInNamespace, kubeServerURL, caData) + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) expectedActions := []coretesting.Action{ - coretesting.NewCreateAction(expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig), + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + coretesting.NewCreateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), } - - // Expect a LoginDiscoveryConfig to be created with the fields from the cluster-info ConfigMap - actualCreatedObject := placeholderClient.Actions()[0].(coretesting.CreateActionImpl).Object - r.Equal(expectedLoginDiscoveryConfig, actualCreatedObject) r.Equal(expectedActions, placeholderClient.Actions()) }) + + when("creating the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "create", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("create failed") + }, + ) + }) + + it("returns the create error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not create logindiscoveryconfig: create failed") + }) + }) }) when("the LoginDiscoveryConfig already exists", func() { when("the LoginDiscoveryConfig is already up to date according to the data in the ConfigMap", func() { it.Before(func() { - // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data matches the ConfigMap's data + _, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) + r.NoError(err) }) - it.Pend("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes", func() { + it("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes/api calls", func() { err := controller.TestSync(t, subject, *controllerContext) r.NoError(err) - r.Empty(placeholderClient.Actions()) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + } + r.Equal(expectedActions, placeholderClient.Actions()) + }) + + when("getting the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "get", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("get failed") + }, + ) + }) + + it("returns the get error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not get logindiscoveryconfig: get failed") + }) }) }) when("the LoginDiscoveryConfig is stale compared to the data in the ConfigMap", func() { it.Before(func() { - // TODO add a fake LoginDiscoveryConfig to the placeholderClient whose data does not match the ConfigMap's data + _, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedLoginDiscoveryConfig.Spec.Server = "https://some-other-server" + err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) + r.NoError(err) }) - it.Pend("updates the existing LoginDiscoveryConfig", func() { - // TODO + it("updates the existing LoginDiscoveryConfig", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.NoError(err) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedActions := []coretesting.Action{ + coretesting.NewGetAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig.Name, + ), + coretesting.NewUpdateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), + } + r.Equal(expectedActions, placeholderClient.Actions()) + }) + + when("updating the LoginDiscoveryConfig fails", func() { + it.Before(func() { + placeholderClient.PrependReactor( + "update", + "logindiscoveryconfigs", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("update failed") + }, + ) + }) + + it("returns the update error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "could not update logindiscoveryconfig: update failed") + }) }) }) }) @@ -181,5 +289,22 @@ func TestRun(t *testing.T) { r.Empty(placeholderClient.Actions()) }) }) + + when("getting the cluster-info ConfigMap in the kube-public namespace fails", func() { + it.Before(func() { + kubeClient.PrependReactor( + "get", + "configmaps", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("get failed") + }, + ) + }) + + it("returns an error", func() { + err := controller.TestSync(t, subject, *controllerContext) + r.EqualError(err, "failed to get cluster-info configmap: get failed") + }) + }) }, spec.Parallel(), spec.Report(report.Terminal{})) } From ee865fe97f44b9d98a22a8cdd61d56992b3de721 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 30 Jul 2020 10:39:38 -0400 Subject: [PATCH 07/17] logindiscovery: add package documentation. Signed-off-by: Andrew Keesler --- internal/controller/logindiscovery/doc.go | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internal/controller/logindiscovery/doc.go diff --git a/internal/controller/logindiscovery/doc.go b/internal/controller/logindiscovery/doc.go new file mode 100644 index 00000000..513f8a1e --- /dev/null +++ b/internal/controller/logindiscovery/doc.go @@ -0,0 +1,7 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +// Package logindiscovery contains controller(s) for reconciling LoginDiscoveryConfig's. +package logindiscovery From 5aebb76146fa5960a476258666b16fad430e6bcd Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 30 Jul 2020 14:34:13 -0700 Subject: [PATCH 08/17] Make the PublisherController use informers Signed-off-by: Andrew Keesler --- go.mod | 2 +- go.sum | 4 +- .../controller/logindiscovery/publisher.go | 56 +++--- .../logindiscovery/publisher_test.go | 161 +++++++++--------- 4 files changed, 119 insertions(+), 104 deletions(-) diff --git a/go.mod b/go.mod index 6786bf68..aeb723dc 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 - github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 + github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200729202601-9b4b6d38494c k8s.io/api v0.19.0-rc.0 diff --git a/go.sum b/go.sum index 3a317f7a..6e6bb53f 100644 --- a/go.sum +++ b/go.sum @@ -530,8 +530,8 @@ github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= -github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499 h1:2NQMYv6IIjcYGkNdezwcY7/MS/3goEZThlz7u6ml9j4= -github.com/suzerain-io/controller-go v0.0.0-20200728175738-b49edda60499/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= +github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f h1:gZ6rAdl+VE9DT0yE52xY/kJZ/hOJYxwtsgGoPr5vItI= +github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9 h1:xnco3XJMrvlwyQJfKoyVPciATvCJ3Y6SY2D8gI2DT2E= github.com/suzerain-io/placeholder-name-api v0.0.0-20200729202220-f1696913d7c9/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= github.com/suzerain-io/placeholder-name-api v0.0.0-20200730131400-4a1da8d7e70b h1:7Fuizf0c3ffqyHj7X4AvXnYNFJHbSgHKjuDxDsxeQ8A= diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index adbeb3cc..d1ba4456 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -12,13 +12,14 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" + corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/tools/clientcmd" "k8s.io/klog/v2" "github.com/suzerain-io/controller-go" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" placeholderclientset "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned" + placeholderv1alpha1informers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions/placeholder/v1alpha1" ) const ( @@ -30,29 +31,46 @@ const ( ) type publisherController struct { - namespace string - kubeClient kubernetes.Interface - placeholderClient placeholderclientset.Interface + namespace string + placeholderClient placeholderclientset.Interface + configMapInformer corev1informers.ConfigMapInformer + loginDiscoveryConfigInformer placeholderv1alpha1informers.LoginDiscoveryConfigInformer } -func NewPublisherController(namespace string, kubeClient kubernetes.Interface, placeholderClient placeholderclientset.Interface) controller.Controller { +func NewPublisherController( + namespace string, + placeholderClient placeholderclientset.Interface, + configMapInformer corev1informers.ConfigMapInformer, + loginDiscoveryConfigInformer placeholderv1alpha1informers.LoginDiscoveryConfigInformer, +) controller.Controller { return controller.New( controller.Config{ Name: "publisher-controller", Syncer: &publisherController{ - namespace: namespace, - kubeClient: kubeClient, - placeholderClient: placeholderClient, + namespace: namespace, + placeholderClient: placeholderClient, + configMapInformer: configMapInformer, + loginDiscoveryConfigInformer: loginDiscoveryConfigInformer, }, }, + controller.WithInformer( + configMapInformer, + controller.FilterFuncs{}, // TODO fix this and write tests + controller.InformerOption{}, + ), + controller.WithInformer( + loginDiscoveryConfigInformer, + controller.FilterFuncs{}, // TODO fix this and write tests + controller.InformerOption{}, + ), ) } func (c *publisherController) Sync(ctx controller.Context) error { - configMap, err := c.kubeClient. - CoreV1(). + configMap, err := c.configMapInformer. + Lister(). ConfigMaps(clusterInfoNamespace). - Get(ctx.Context, clusterInfoName, metav1.GetOptions{}) + Get(clusterInfoName) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return fmt.Errorf("failed to get %s configmap: %w", clusterInfoName, err) @@ -103,20 +121,18 @@ func (c *publisherController) createOrUpdateLoginDiscoveryConfig( ctx context.Context, discoveryConfig *placeholderv1alpha1.LoginDiscoveryConfig, ) error { - loginDiscoveryConfigs := c.placeholderClient. - PlaceholderV1alpha1(). - LoginDiscoveryConfigs(c.namespace) - - existingDiscoveryConfig, err := loginDiscoveryConfigs.Get( - ctx, - discoveryConfig.Name, - metav1.GetOptions{}, - ) + existingDiscoveryConfig, err := c.loginDiscoveryConfigInformer. + Lister(). + LoginDiscoveryConfigs(c.namespace). + Get(discoveryConfig.Name) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return fmt.Errorf("could not get logindiscoveryconfig: %w", err) } + loginDiscoveryConfigs := c.placeholderClient. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(c.namespace) if notFound { if _, err := loginDiscoveryConfigs.Create( ctx, diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index eff0dacb..2099c722 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -19,12 +19,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" "github.com/suzerain-io/controller-go" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" placeholderfake "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned/fake" + placeholderinformers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions" ) func TestRun(t *testing.T) { @@ -34,11 +36,14 @@ func TestRun(t *testing.T) { var r *require.Assertions var subject controller.Controller - var kubeClient *kubernetesfake.Clientset - var placeholderClient *placeholderfake.Clientset + var kubeInformerClient *kubernetesfake.Clientset + var placeholderInformerClient *placeholderfake.Clientset + var kubeInformers kubeinformers.SharedInformerFactory + var placeholderInformers placeholderinformers.SharedInformerFactory + var placeholderAPIClient *placeholderfake.Clientset var timeoutContext context.Context var timeoutContextCancel context.CancelFunc - var controllerContext *controller.Context + var syncContext *controller.Context var expectedLoginDiscoveryConfig = func(expectedNamespace, expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *placeholderv1alpha1.LoginDiscoveryConfig) { expectedLoginDiscoveryConfigGVR := schema.GroupVersionResource{ @@ -59,13 +64,34 @@ func TestRun(t *testing.T) { return expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig } + // Defer starting the informers until the last possible moment so that the + // nested Before's can keep adding things to the informer caches. + var startInformersAndController = func() { + // Must start informers before calling TestRunSynchronously() + kubeInformers.Start(timeoutContext.Done()) + placeholderInformers.Start(timeoutContext.Done()) + controller.TestRunSynchronously(t, subject) + } + it.Before(func() { r = require.New(t) - kubeClient = kubernetesfake.NewSimpleClientset() - placeholderClient = placeholderfake.NewSimpleClientset() + timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) - subject = NewPublisherController(installedInNamespace, kubeClient, placeholderClient) - controllerContext = &controller.Context{ + + kubeInformerClient = kubernetesfake.NewSimpleClientset() + kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0) + placeholderAPIClient = placeholderfake.NewSimpleClientset() + placeholderInformerClient = placeholderfake.NewSimpleClientset() + placeholderInformers = placeholderinformers.NewSharedInformerFactory(placeholderInformerClient, 0) + + subject = NewPublisherController( + installedInNamespace, + placeholderAPIClient, + kubeInformers.Core().V1().ConfigMaps(), + placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), + ) + + syncContext = &controller.Context{ Context: timeoutContext, Name: subject.Name(), Key: controller.Key{ @@ -75,14 +101,17 @@ func TestRun(t *testing.T) { } }) + it.After(func() { + timeoutContextCancel() + }) + when("there is a cluster-info ConfigMap in the kube-public namespace", func() { const caData = "c29tZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YQo=" // "some-certificate-authority-data" base64 encoded const kubeServerURL = "https://some-server" - var clusterInfoConfigMap *corev1.ConfigMap when("the ConfigMap has the expected `kubeconfig` top-level data key", func() { it.Before(func() { - clusterInfoConfigMap = &corev1.ConfigMap{ + clusterInfoConfigMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, // Note that go fmt puts tabs in our file, which we must remove from our configmap yaml below. Data: map[string]string{ @@ -97,15 +126,14 @@ func TestRun(t *testing.T) { "uninteresting-key": "uninteresting-value", }, } - err := kubeClient.Tracker().Add(clusterInfoConfigMap) + err := kubeInformerClient.Tracker().Add(clusterInfoConfigMap) r.NoError(err) }) when("the LoginDiscoveryConfig does not already exist", func() { - it("creates a LoginDiscoveryConfig", func() { - defer timeoutContextCancel() - - err := controller.TestSync(t, subject, *controllerContext) + it.Focus("creates a LoginDiscoveryConfig", func() { + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.NoError(err) expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( @@ -113,24 +141,22 @@ func TestRun(t *testing.T) { kubeServerURL, caData, ) - expectedActions := []coretesting.Action{ - coretesting.NewGetAction( - expectedLoginDiscoveryConfigGVR, - installedInNamespace, - expectedLoginDiscoveryConfig.Name, - ), - coretesting.NewCreateAction( - expectedLoginDiscoveryConfigGVR, - installedInNamespace, - expectedLoginDiscoveryConfig, - ), - } - r.Equal(expectedActions, placeholderClient.Actions()) + + r.Equal( + []coretesting.Action{ + coretesting.NewCreateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), + }, + placeholderAPIClient.Actions(), + ) }) when("creating the LoginDiscoveryConfig fails", func() { it.Before(func() { - placeholderClient.PrependReactor( + placeholderAPIClient.PrependReactor( "create", "logindiscoveryconfigs", func(_ coretesting.Action) (bool, runtime.Object, error) { @@ -140,7 +166,8 @@ func TestRun(t *testing.T) { }) it("returns the create error", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.EqualError(err, "could not create logindiscoveryconfig: create failed") }) }) @@ -154,44 +181,16 @@ func TestRun(t *testing.T) { kubeServerURL, caData, ) - err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) + err := placeholderInformerClient.Tracker().Add(expectedLoginDiscoveryConfig) r.NoError(err) }) it("does not update the LoginDiscoveryConfig to avoid unnecessary etcd writes/api calls", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.NoError(err) - expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( - installedInNamespace, - kubeServerURL, - caData, - ) - expectedActions := []coretesting.Action{ - coretesting.NewGetAction( - expectedLoginDiscoveryConfigGVR, - installedInNamespace, - expectedLoginDiscoveryConfig.Name, - ), - } - r.Equal(expectedActions, placeholderClient.Actions()) - }) - - when("getting the LoginDiscoveryConfig fails", func() { - it.Before(func() { - placeholderClient.PrependReactor( - "get", - "logindiscoveryconfigs", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("get failed") - }, - ) - }) - - it("returns the get error", func() { - err := controller.TestSync(t, subject, *controllerContext) - r.EqualError(err, "could not get logindiscoveryconfig: get failed") - }) + r.Empty(placeholderAPIClient.Actions()) }) }) @@ -203,12 +202,13 @@ func TestRun(t *testing.T) { caData, ) expectedLoginDiscoveryConfig.Spec.Server = "https://some-other-server" - err := placeholderClient.Tracker().Add(expectedLoginDiscoveryConfig) - r.NoError(err) + r.NoError(placeholderInformerClient.Tracker().Add(expectedLoginDiscoveryConfig)) + r.NoError(placeholderAPIClient.Tracker().Add(expectedLoginDiscoveryConfig)) }) it("updates the existing LoginDiscoveryConfig", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.NoError(err) expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( @@ -217,23 +217,18 @@ func TestRun(t *testing.T) { caData, ) expectedActions := []coretesting.Action{ - coretesting.NewGetAction( - expectedLoginDiscoveryConfigGVR, - installedInNamespace, - expectedLoginDiscoveryConfig.Name, - ), coretesting.NewUpdateAction( expectedLoginDiscoveryConfigGVR, installedInNamespace, expectedLoginDiscoveryConfig, ), } - r.Equal(expectedActions, placeholderClient.Actions()) + r.Equal(expectedActions, placeholderAPIClient.Actions()) }) when("updating the LoginDiscoveryConfig fails", func() { it.Before(func() { - placeholderClient.PrependReactor( + placeholderAPIClient.PrependReactor( "update", "logindiscoveryconfigs", func(_ coretesting.Action) (bool, runtime.Object, error) { @@ -243,7 +238,8 @@ func TestRun(t *testing.T) { }) it("returns the update error", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.EqualError(err, "could not update logindiscoveryconfig: update failed") }) }) @@ -253,20 +249,21 @@ func TestRun(t *testing.T) { when("the ConfigMap is missing the expected `kubeconfig` top-level data key", func() { it.Before(func() { - clusterInfoConfigMap = &corev1.ConfigMap{ + clusterInfoConfigMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}, Data: map[string]string{ "these are not the droids you're looking for": "uninteresting-value", }, } - err := kubeClient.Tracker().Add(clusterInfoConfigMap) + err := kubeInformerClient.Tracker().Add(clusterInfoConfigMap) r.NoError(err) }) it("keeps waiting for it to exist", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.NoError(err) - r.Empty(placeholderClient.Actions()) + r.Empty(placeholderAPIClient.Actions()) }) }) }) @@ -279,20 +276,21 @@ func TestRun(t *testing.T) { Namespace: "kube-public", }, } - err := kubeClient.Tracker().Add(unrelatedConfigMap) + err := kubeInformerClient.Tracker().Add(unrelatedConfigMap) r.NoError(err) }) it("keeps waiting for one", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.NoError(err) - r.Empty(placeholderClient.Actions()) + r.Empty(placeholderAPIClient.Actions()) }) }) when("getting the cluster-info ConfigMap in the kube-public namespace fails", func() { it.Before(func() { - kubeClient.PrependReactor( + kubeInformerClient.PrependReactor( "get", "configmaps", func(_ coretesting.Action) (bool, runtime.Object, error) { @@ -302,7 +300,8 @@ func TestRun(t *testing.T) { }) it("returns an error", func() { - err := controller.TestSync(t, subject, *controllerContext) + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) r.EqualError(err, "failed to get cluster-info configmap: get failed") }) }) From 733f80b7ae9b4be805e5ecef7e14f864c9228e67 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 30 Jul 2020 17:16:09 -0700 Subject: [PATCH 09/17] Apply filters to PublisherController - Ask the controller package to only call the Sync() method for the specific objects in which this controller is interested --- .../controller/logindiscovery/publisher.go | 28 ++- .../logindiscovery/publisher_test.go | 174 +++++++++++++++--- 2 files changed, 177 insertions(+), 25 deletions(-) diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index d1ba4456..0d6d04b8 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -30,6 +30,25 @@ const ( configName = "placeholder-name-config" ) +func nameAndNamespaceExactMatchFilterFactory(name, namespace string) controller.FilterFuncs { + objMatchesFunc := func(obj metav1.Object) bool { + return obj.GetName() == name && obj.GetNamespace() == namespace + } + return controller.FilterFuncs{ + AddFunc: objMatchesFunc, + UpdateFunc: func(oldObj, newObj metav1.Object) bool { + return objMatchesFunc(oldObj) || objMatchesFunc(newObj) + }, + DeleteFunc: objMatchesFunc, + } +} + +// Same signature as controller.WithInformer(). +type withInformerOptionFunc func( + getter controller.InformerGetter, + filter controller.Filter, + opt controller.InformerOption) controller.Option + type publisherController struct { namespace string placeholderClient placeholderclientset.Interface @@ -42,6 +61,7 @@ func NewPublisherController( placeholderClient placeholderclientset.Interface, configMapInformer corev1informers.ConfigMapInformer, loginDiscoveryConfigInformer placeholderv1alpha1informers.LoginDiscoveryConfigInformer, + withInformer withInformerOptionFunc, ) controller.Controller { return controller.New( controller.Config{ @@ -53,14 +73,14 @@ func NewPublisherController( loginDiscoveryConfigInformer: loginDiscoveryConfigInformer, }, }, - controller.WithInformer( + withInformer( configMapInformer, - controller.FilterFuncs{}, // TODO fix this and write tests + nameAndNamespaceExactMatchFilterFactory(clusterInfoName, clusterInfoNamespace), controller.InformerOption{}, ), - controller.WithInformer( + withInformer( loginDiscoveryConfigInformer, - controller.FilterFuncs{}, // TODO fix this and write tests + nameAndNamespaceExactMatchFilterFactory(configName, namespace), controller.InformerOption{}, ), ) diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index 2099c722..8d90354c 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -29,8 +29,157 @@ import ( placeholderinformers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions" ) -func TestRun(t *testing.T) { - spec.Run(t, "publisher", func(t *testing.T, when spec.G, it spec.S) { +type ObservableWithInformerOption struct { + InformerToFilterMap map[controller.InformerGetter]controller.Filter +} + +func NewObservableWithInformerOption() *ObservableWithInformerOption { + return &ObservableWithInformerOption{ + InformerToFilterMap: make(map[controller.InformerGetter]controller.Filter), + } +} + +func (owi *ObservableWithInformerOption) WithInformer( + getter controller.InformerGetter, + filter controller.Filter, + opt controller.InformerOption) controller.Option { + owi.InformerToFilterMap[getter] = filter + return controller.WithInformer(getter, filter, opt) +} + +func TestInformerFilters(t *testing.T) { + spec.Run(t, "informer filters", func(t *testing.T, when spec.G, it spec.S) { + const installedInNamespace = "some-namespace" + + var r *require.Assertions + var observableWithInformerOption *ObservableWithInformerOption + var configMapInformerFilter controller.Filter + var loginDiscoveryConfigInformerFilter controller.Filter + + it.Before(func() { + r = require.New(t) + observableWithInformerOption = NewObservableWithInformerOption() + configMapInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().ConfigMaps() + loginDiscoveryConfigInformer := placeholderinformers.NewSharedInformerFactory(nil, 0).Placeholder().V1alpha1().LoginDiscoveryConfigs() + _ = NewPublisherController( + installedInNamespace, + nil, + configMapInformer, + loginDiscoveryConfigInformer, + observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters + ) + configMapInformerFilter = observableWithInformerOption.InformerToFilterMap[configMapInformer] + loginDiscoveryConfigInformerFilter = observableWithInformerOption.InformerToFilterMap[loginDiscoveryConfigInformer] + }) + + when("watching ConfigMap objects", func() { + var subject controller.Filter + var target, wrongNamespace, wrongName, unrelated *corev1.ConfigMap + + it.Before(func() { + subject = configMapInformerFilter + target = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "kube-public"}} + wrongNamespace = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cluster-info", Namespace: "wrong-namespace"}} + wrongName = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "kube-public"}} + unrelated = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}} + }) + + when("the target ConfigMap changes", func() { + it("returns true to trigger the sync method", func() { + r.True(subject.Add(target)) + r.True(subject.Update(target, unrelated)) + r.True(subject.Update(unrelated, target)) + r.True(subject.Delete(target)) + }) + }) + + when("a ConfigMap from another namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongNamespace)) + r.False(subject.Update(wrongNamespace, unrelated)) + r.False(subject.Update(unrelated, wrongNamespace)) + r.False(subject.Delete(wrongNamespace)) + }) + }) + + when("a ConfigMap with a different name changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongName)) + r.False(subject.Update(wrongName, unrelated)) + r.False(subject.Update(unrelated, wrongName)) + r.False(subject.Delete(wrongName)) + }) + }) + + when("a ConfigMap with a different name and a different namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(unrelated)) + r.False(subject.Update(unrelated, unrelated)) + r.False(subject.Delete(unrelated)) + }) + }) + }) + + when("watching LoginDiscoveryConfig objects", func() { + var subject controller.Filter + var target, wrongNamespace, wrongName, unrelated *placeholderv1alpha1.LoginDiscoveryConfig + + it.Before(func() { + subject = loginDiscoveryConfigInformerFilter + target = &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "placeholder-name-config", Namespace: installedInNamespace}, + } + wrongNamespace = &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "placeholder-name-config", Namespace: "wrong-namespace"}, + } + wrongName = &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: installedInNamespace}, + } + unrelated = &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}, + } + }) + + when("the target LoginDiscoveryConfig changes", func() { + it("returns true to trigger the sync method", func() { + r.True(subject.Add(target)) + r.True(subject.Update(target, unrelated)) + r.True(subject.Update(unrelated, target)) + r.True(subject.Delete(target)) + }) + }) + + when("a LoginDiscoveryConfig from another namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongNamespace)) + r.False(subject.Update(wrongNamespace, unrelated)) + r.False(subject.Update(unrelated, wrongNamespace)) + r.False(subject.Delete(wrongNamespace)) + }) + }) + + when("a LoginDiscoveryConfig with a different name changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongName)) + r.False(subject.Update(wrongName, unrelated)) + r.False(subject.Update(unrelated, wrongName)) + r.False(subject.Delete(wrongName)) + }) + }) + + when("a LoginDiscoveryConfig with a different name and a different namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(unrelated)) + r.False(subject.Update(unrelated, unrelated)) + r.False(subject.Delete(unrelated)) + }) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) +} + +func TestSync(t *testing.T) { + spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { const installedInNamespace = "some-namespace" var r *require.Assertions @@ -89,6 +238,7 @@ func TestRun(t *testing.T) { placeholderAPIClient, kubeInformers.Core().V1().ConfigMaps(), placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), + controller.WithInformer, ) syncContext = &controller.Context{ @@ -131,7 +281,7 @@ func TestRun(t *testing.T) { }) when("the LoginDiscoveryConfig does not already exist", func() { - it.Focus("creates a LoginDiscoveryConfig", func() { + it("creates a LoginDiscoveryConfig", func() { startInformersAndController() err := controller.TestSync(t, subject, *syncContext) r.NoError(err) @@ -287,23 +437,5 @@ func TestRun(t *testing.T) { r.Empty(placeholderAPIClient.Actions()) }) }) - - when("getting the cluster-info ConfigMap in the kube-public namespace fails", func() { - it.Before(func() { - kubeInformerClient.PrependReactor( - "get", - "configmaps", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("get failed") - }, - ) - }) - - it("returns an error", func() { - startInformersAndController() - err := controller.TestSync(t, subject, *syncContext) - r.EqualError(err, "failed to get cluster-info configmap: get failed") - }) - }) }, spec.Parallel(), spec.Report(report.Terminal{})) } From 52546fad901f272127e55912de5fe77f200c5a56 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Fri, 31 Jul 2020 12:08:07 -0400 Subject: [PATCH 10/17] WIP: start on publisher controller integration --- deploy/crd.yaml | 4 +- deploy/rbac.yaml | 27 ++++ internal/apiserver/apiserver.go | 20 ++- .../controller/logindiscovery/publisher.go | 9 +- internal/server/server.go | 85 +++++++++++-- test/integration/logindiscoveryconfig_test.go | 116 ++++++++++++++++++ 6 files changed, 242 insertions(+), 19 deletions(-) create mode 100644 test/integration/logindiscoveryconfig_test.go diff --git a/deploy/crd.yaml b/deploy/crd.yaml index de814775..a859de09 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -15,9 +15,9 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: logindiscoveryconfigs.suzerain-io.github.io + name: logindiscoveryconfigs.placeholder.suzerain-io.github.io spec: - group: suzerain-io.github.io + group: placeholder.suzerain-io.github.io versions: #! Any changes to these schemas should also be reflected in the types.go file(s) #! in https://github.com/suzerain-io/placeholder-name-api/tree/main/pkg/apis/placeholder diff --git a/deploy/rbac.yaml b/deploy/rbac.yaml index e3d0858a..ba6cec00 100644 --- a/deploy/rbac.yaml +++ b/deploy/rbac.yaml @@ -38,6 +38,9 @@ rules: - apiGroups: [""] resources: [services] verbs: [create, get, list, patch, update, watch] + - apiGroups: [placeholder.suzerain-io.github.io] + resources: [logindiscoveryconfigs] + verbs: [create, get, list, update, watch] --- kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 @@ -108,3 +111,27 @@ roleRef: #! give permissions for a special configmap of CA bundles that is needed by aggregated api servers name: extension-apiserver-authentication-reader apiGroup: rbac.authorization.k8s.io +--- +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: #@ data.values.app_name + "-cluster-info-lister-watcher-role" + namespace: kube-public +rules: + - apiGroups: [""] + resources: [configmaps] + verbs: [list, watch] #! TODO: do we neeed a get here for the controller? +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: #@ data.values.app_name + "-cluster-info-lister-watcher-role-binding" + namespace: kube-public +subjects: + - kind: ServiceAccount + name: #@ data.values.app_name + "-service-account" + namespace: #@ data.values.namespace +roleRef: + kind: Role + name: #@ data.values.app_name + "-cluster-info-lister-watcher-role" + apiGroup: rbac.authorization.k8s.io diff --git a/internal/apiserver/apiserver.go b/internal/apiserver/apiserver.go index 2fb42b0f..338e740b 100644 --- a/internal/apiserver/apiserver.go +++ b/internal/apiserver/apiserver.go @@ -6,6 +6,7 @@ SPDX-License-Identifier: Apache-2.0 package apiserver import ( + "context" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -56,8 +57,9 @@ type Config struct { } type ExtraConfig struct { - Webhook authenticator.Token - Issuer loginrequest.CertIssuer + Webhook authenticator.Token + Issuer loginrequest.CertIssuer + StartControllersPostStartHook func(ctx context.Context) } type PlaceHolderServer struct { @@ -122,9 +124,17 @@ func (c completedConfig) New() (*PlaceHolderServer, error) { return nil, fmt.Errorf("install API group error: %w", err) } - s.GenericAPIServer.AddPostStartHookOrDie("place-holder-post-start-hook", - func(context genericapiserver.PostStartHookContext) error { - klog.InfoS("post start hook", "foo", "bar") + s.GenericAPIServer.AddPostStartHookOrDie("start-controllers", + func(postStartContext genericapiserver.PostStartHookContext) error { + klog.InfoS("post start hook") + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + <-postStartContext.StopCh + cancel() + }() + c.ExtraConfig.StartControllersPostStartHook(ctx) + return nil }, ) diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 0d6d04b8..0725437b 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -23,8 +23,9 @@ import ( ) const ( + ClusterInfoNamespace = "kube-public" + clusterInfoName = "cluster-info" - clusterInfoNamespace = "kube-public" clusterInfoConfigMapKey = "kubeconfig" configName = "placeholder-name-config" @@ -75,7 +76,7 @@ func NewPublisherController( }, withInformer( configMapInformer, - nameAndNamespaceExactMatchFilterFactory(clusterInfoName, clusterInfoNamespace), + nameAndNamespaceExactMatchFilterFactory(clusterInfoName, ClusterInfoNamespace), controller.InformerOption{}, ), withInformer( @@ -89,7 +90,7 @@ func NewPublisherController( func (c *publisherController) Sync(ctx controller.Context) error { configMap, err := c.configMapInformer. Lister(). - ConfigMaps(clusterInfoNamespace). + ConfigMaps(ClusterInfoNamespace). Get(clusterInfoName) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { @@ -99,7 +100,7 @@ func (c *publisherController) Sync(ctx controller.Context) error { klog.InfoS( "could not find config map", "configmap", - klog.KRef(clusterInfoNamespace, clusterInfoName), + klog.KRef(ClusterInfoNamespace, clusterInfoName), ) return nil } diff --git a/internal/server/server.go b/internal/server/server.go index 442e4fd5..550ba745 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -25,20 +25,30 @@ import ( "k8s.io/apiserver/pkg/server/dynamiccertificates" genericoptions "k8s.io/apiserver/pkg/server/options" "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook" + k8sinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" restclient "k8s.io/client-go/rest" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" aggregationv1client "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" + "github.com/suzerain-io/controller-go" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + placeholderclientset "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned" + placeholderinformers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions" "github.com/suzerain-io/placeholder-name/internal/apiserver" "github.com/suzerain-io/placeholder-name/internal/autoregistration" "github.com/suzerain-io/placeholder-name/internal/certauthority" + "github.com/suzerain-io/placeholder-name/internal/controller/logindiscovery" "github.com/suzerain-io/placeholder-name/internal/downward" "github.com/suzerain-io/placeholder-name/pkg/config" ) +// TODO(akeesler): what should these controller settings be? +const ( + defaultWorkers = 3 + defaultResync = 20 * time.Minute +) + // App is an object that represents the placeholder-name-server application. type App struct { cmd *cobra.Command @@ -94,7 +104,15 @@ authenticating to the Kubernetes API.`, return fmt.Errorf("could not initialize Kubernetes client: %w", err) } - return a.run(ctx, k8s.CoreV1(), aggregation) + // Connect to the placeholder API. + // I think we can't use protobuf encoding here because we are using CRDs + // (for which protobuf encoding is not supported). + placeholder, err := placeholderclientset.NewForConfig(kubeConfig) + if err != nil { + return fmt.Errorf("could not initialize placeholder client: %w", err) + } + + return a.run(ctx, k8s, aggregation, placeholder) }, Args: cobra.NoArgs, } @@ -143,8 +161,9 @@ func (a *App) Run() error { func (a *App) run( ctx context.Context, - k8s corev1client.CoreV1Interface, + k8s kubernetes.Interface, aggregation aggregationv1client.Interface, + placeholder placeholderclientset.Interface, ) error { cfg, err := config.FromPath(a.configPath) if err != nil { @@ -213,7 +232,7 @@ func (a *App) run( }, } if err := autoregistration.Setup(ctx, autoregistration.SetupOptions{ - CoreV1: k8s, + CoreV1: k8s.CoreV1(), AggregationV1: aggregation, Namespace: podinfo.Namespace, ServiceTemplate: service, @@ -222,7 +241,13 @@ func (a *App) run( return fmt.Errorf("could not register API service: %w", err) } - apiServerConfig, err := a.ConfigServer(cert, webhookTokenAuthenticator, clientCA) + cmrf := wireControllerManagerRunFunc(podinfo, k8s, placeholder) + apiServerConfig, err := a.configServer( + cert, + webhookTokenAuthenticator, + clientCA, + cmrf, + ) if err != nil { return err } @@ -235,7 +260,12 @@ func (a *App) run( return server.GenericAPIServer.PrepareRun().Run(ctx.Done()) } -func (a *App) ConfigServer(cert *tls.Certificate, webhookTokenAuthenticator *webhook.WebhookTokenAuthenticator, ca *certauthority.CA) (*apiserver.Config, error) { +func (a *App) configServer( + cert *tls.Certificate, + webhookTokenAuthenticator *webhook.WebhookTokenAuthenticator, + ca *certauthority.CA, + startControllersPostStartHook func(context.Context), +) (*apiserver.Config, error) { provider, err := createStaticCertKeyProvider(cert) if err != nil { return nil, fmt.Errorf("could not create static cert key provider: %w", err) @@ -250,8 +280,9 @@ func (a *App) ConfigServer(cert *tls.Certificate, webhookTokenAuthenticator *web apiServerConfig := &apiserver.Config{ GenericConfig: serverConfig, ExtraConfig: apiserver.ExtraConfig{ - Webhook: webhookTokenAuthenticator, - Issuer: ca, + Webhook: webhookTokenAuthenticator, + Issuer: ca, + StartControllersPostStartHook: startControllersPostStartHook, }, } return apiServerConfig, nil @@ -290,3 +321,41 @@ func createStaticCertKeyProvider(cert *tls.Certificate) (dynamiccertificates.Cer return dynamiccertificates.NewStaticCertKeyContent("some-name???", certChainPEM, privateKeyPEM) } + +func wireControllerManagerRunFunc( + podinfo *downward.PodInfo, + k8s kubernetes.Interface, + placeholder placeholderclientset.Interface, +) func(ctx context.Context) { + k8sInformers := k8sinformers.NewSharedInformerFactoryWithOptions( + k8s, + defaultResync, + k8sinformers.WithNamespace( + logindiscovery.ClusterInfoNamespace, + ), + ) + placeholderInformers := placeholderinformers.NewSharedInformerFactoryWithOptions( + placeholder, + defaultResync, + placeholderinformers.WithNamespace( + "integration", // TODO(akeesler): unhardcode this. + ), + ) + cm := controller. + NewManager(). + WithController( + logindiscovery.NewPublisherController( + podinfo.Namespace, + placeholder, + k8sInformers.Core().V1().ConfigMaps(), + placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), + controller.WithInformer, + ), + defaultWorkers, + ) + return func(ctx context.Context) { + k8sInformers.Start(ctx.Done()) + placeholderInformers.Start(ctx.Done()) + go cm.Start(ctx) + } +} diff --git a/test/integration/logindiscoveryconfig_test.go b/test/integration/logindiscoveryconfig_test.go new file mode 100644 index 00000000..043c8298 --- /dev/null +++ b/test/integration/logindiscoveryconfig_test.go @@ -0,0 +1,116 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package integration + +import ( + "context" + "encoding/base64" + "os" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/rest" + + placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + "github.com/suzerain-io/placeholder-name/test/library" +) + +func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { + namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") + require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") + + client := library.NewPlaceholderNameClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // TODO(akeesler): is there a race here between this test running and the + // placeholder-name-server creating the CR? + + config := library.NewClientConfig(t) + expectedLDC := getExpectedLDC(namespaceName, config) + configList, err := client. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(namespaceName). + List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, configList.Items, 1) + require.Equal(t, expectedLDC, configList.Items[0]) +} + +func TestReconcilingLoginDiscoveryConfig(t *testing.T) { + t.Skip() + + namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") + require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") + + client := library.NewPlaceholderNameClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // TODO(akeesler): is there a race here between this test running and the + // placeholder-name-server creating the CR? + + w, err := client. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(namespaceName). + Watch(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + err = client. + PlaceholderV1alpha1(). + LoginDiscoveryConfigs(namespaceName). + Delete(ctx, "placeholder-name-config", metav1.DeleteOptions{}) + require.NoError(t, err) + + config := library.NewClientConfig(t) + expectedLDC := getExpectedLDC(namespaceName, config) + received := func(et watch.EventType, o runtime.Object) func() bool { + return func() bool { + select { + case e := <-w.ResultChan(): + require.Equal(t, et, e.Type) + require.Equal(t, o, e.Object) + return true + default: + return false + } + } + } + require.Eventually( + t, + received(watch.Deleted, expectedLDC), + time.Second, + 3*time.Second, + ) + require.Eventually( + t, + received(watch.Added, expectedLDC), + time.Second, + 3*time.Second, + ) +} + +func getExpectedLDC( + namespaceName string, + config *rest.Config, +) *placeholderv1alpha1.LoginDiscoveryConfig { + return &placeholderv1alpha1.LoginDiscoveryConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "placeholder-name-config", + Namespace: namespaceName, + }, + Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Server: config.Host, + CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), + }, + } +} From 2aa80e357679120e24dfc277cd2f58c9955f3d15 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 31 Jul 2020 14:35:20 -0700 Subject: [PATCH 11/17] More WIP for the publisher controller --- deploy/rbac.yaml | 2 +- internal/apiserver/apiserver.go | 2 +- internal/server/server.go | 44 ++++++----- test/integration/logindiscoveryconfig_test.go | 74 +++++-------------- test/integration/loginrequest_test.go | 55 ++++++++------ 5 files changed, 74 insertions(+), 103 deletions(-) diff --git a/deploy/rbac.yaml b/deploy/rbac.yaml index ba6cec00..fd67592f 100644 --- a/deploy/rbac.yaml +++ b/deploy/rbac.yaml @@ -120,7 +120,7 @@ metadata: rules: - apiGroups: [""] resources: [configmaps] - verbs: [list, watch] #! TODO: do we neeed a get here for the controller? + verbs: [list, watch] --- kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/internal/apiserver/apiserver.go b/internal/apiserver/apiserver.go index 338e740b..d7b6d639 100644 --- a/internal/apiserver/apiserver.go +++ b/internal/apiserver/apiserver.go @@ -126,7 +126,7 @@ func (c completedConfig) New() (*PlaceHolderServer, error) { s.GenericAPIServer.AddPostStartHookOrDie("start-controllers", func(postStartContext genericapiserver.PostStartHookContext) error { - klog.InfoS("post start hook") + klog.InfoS("start-controllers post start hook starting") ctx, cancel := context.WithCancel(context.Background()) go func() { diff --git a/internal/server/server.go b/internal/server/server.go index 550ba745..a2dc9664 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -43,10 +43,9 @@ import ( "github.com/suzerain-io/placeholder-name/pkg/config" ) -// TODO(akeesler): what should these controller settings be? const ( - defaultWorkers = 3 - defaultResync = 20 * time.Minute + singletonWorker = 1 + defaultResyncInterval = 3 * time.Minute ) // App is an object that represents the placeholder-name-server application. @@ -93,13 +92,13 @@ authenticating to the Kubernetes API.`, protoKubeConfig := createProtoKubeConfig(kubeConfig) // Connect to the core Kubernetes API. - k8s, err := kubernetes.NewForConfig(protoKubeConfig) + k8sClient, err := kubernetes.NewForConfig(protoKubeConfig) if err != nil { return fmt.Errorf("could not initialize Kubernetes client: %w", err) } // Connect to the Kubernetes aggregation API. - aggregation, err := aggregationv1client.NewForConfig(protoKubeConfig) + aggregationClient, err := aggregationv1client.NewForConfig(protoKubeConfig) if err != nil { return fmt.Errorf("could not initialize Kubernetes client: %w", err) } @@ -107,12 +106,12 @@ authenticating to the Kubernetes API.`, // Connect to the placeholder API. // I think we can't use protobuf encoding here because we are using CRDs // (for which protobuf encoding is not supported). - placeholder, err := placeholderclientset.NewForConfig(kubeConfig) + placeholderClient, err := placeholderclientset.NewForConfig(kubeConfig) if err != nil { return fmt.Errorf("could not initialize placeholder client: %w", err) } - return a.run(ctx, k8s, aggregation, placeholder) + return a.run(ctx, k8sClient, aggregationClient, placeholderClient) }, Args: cobra.NoArgs, } @@ -161,9 +160,9 @@ func (a *App) Run() error { func (a *App) run( ctx context.Context, - k8s kubernetes.Interface, - aggregation aggregationv1client.Interface, - placeholder placeholderclientset.Interface, + k8sClient kubernetes.Interface, + aggregationClient aggregationv1client.Interface, + placeholderClient placeholderclientset.Interface, ) error { cfg, err := config.FromPath(a.configPath) if err != nil { @@ -185,6 +184,7 @@ func (a *App) run( if err != nil { return fmt.Errorf("could not read pod metadata: %w", err) } + serverInstallationNamespace := podinfo.Namespace // TODO use the postStart hook to generate certs? @@ -196,7 +196,7 @@ func (a *App) run( const serviceName = "placeholder-name-api" cert, err := apiCA.Issue( - pkix.Name{CommonName: serviceName + "." + podinfo.Namespace + ".svc"}, + pkix.Name{CommonName: serviceName + "." + serverInstallationNamespace + ".svc"}, []string{}, 24*365*time.Hour, ) @@ -232,16 +232,16 @@ func (a *App) run( }, } if err := autoregistration.Setup(ctx, autoregistration.SetupOptions{ - CoreV1: k8s.CoreV1(), - AggregationV1: aggregation, - Namespace: podinfo.Namespace, + CoreV1: k8sClient.CoreV1(), + AggregationV1: aggregationClient, + Namespace: serverInstallationNamespace, ServiceTemplate: service, APIServiceTemplate: apiService, }); err != nil { return fmt.Errorf("could not register API service: %w", err) } - cmrf := wireControllerManagerRunFunc(podinfo, k8s, placeholder) + cmrf := wireControllerManagerRunFunc(serverInstallationNamespace, k8sClient, placeholderClient) apiServerConfig, err := a.configServer( cert, webhookTokenAuthenticator, @@ -323,35 +323,33 @@ func createStaticCertKeyProvider(cert *tls.Certificate) (dynamiccertificates.Cer } func wireControllerManagerRunFunc( - podinfo *downward.PodInfo, + serverInstallationNamespace string, k8s kubernetes.Interface, placeholder placeholderclientset.Interface, ) func(ctx context.Context) { k8sInformers := k8sinformers.NewSharedInformerFactoryWithOptions( k8s, - defaultResync, + defaultResyncInterval, k8sinformers.WithNamespace( logindiscovery.ClusterInfoNamespace, ), ) placeholderInformers := placeholderinformers.NewSharedInformerFactoryWithOptions( placeholder, - defaultResync, - placeholderinformers.WithNamespace( - "integration", // TODO(akeesler): unhardcode this. - ), + defaultResyncInterval, + placeholderinformers.WithNamespace(serverInstallationNamespace), ) cm := controller. NewManager(). WithController( logindiscovery.NewPublisherController( - podinfo.Namespace, + serverInstallationNamespace, placeholder, k8sInformers.Core().V1().ConfigMaps(), placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), controller.WithInformer, ), - defaultWorkers, + singletonWorker, ) return func(ctx context.Context) { k8sInformers.Start(ctx.Done()) diff --git a/test/integration/logindiscoveryconfig_test.go b/test/integration/logindiscoveryconfig_test.go index 043c8298..8f37b5b4 100644 --- a/test/integration/logindiscoveryconfig_test.go +++ b/test/integration/logindiscoveryconfig_test.go @@ -14,8 +14,6 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/rest" placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" @@ -31,23 +29,18 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - // TODO(akeesler): is there a race here between this test running and the - // placeholder-name-server creating the CR? - config := library.NewClientConfig(t) - expectedLDC := getExpectedLDC(namespaceName, config) + expectedLDCSpec := expectedLDCSpec(config) configList, err := client. PlaceholderV1alpha1(). LoginDiscoveryConfigs(namespaceName). List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Len(t, configList.Items, 1) - require.Equal(t, expectedLDC, configList.Items[0]) + require.Equal(t, expectedLDCSpec, &configList.Items[0].Spec) } func TestReconcilingLoginDiscoveryConfig(t *testing.T) { - t.Skip() - namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") @@ -56,61 +49,32 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - // TODO(akeesler): is there a race here between this test running and the - // placeholder-name-server creating the CR? - - w, err := client. - PlaceholderV1alpha1(). - LoginDiscoveryConfigs(namespaceName). - Watch(ctx, metav1.ListOptions{}) - require.NoError(t, err) - - err = client. + err := client. PlaceholderV1alpha1(). LoginDiscoveryConfigs(namespaceName). Delete(ctx, "placeholder-name-config", metav1.DeleteOptions{}) require.NoError(t, err) config := library.NewClientConfig(t) - expectedLDC := getExpectedLDC(namespaceName, config) - received := func(et watch.EventType, o runtime.Object) func() bool { - return func() bool { - select { - case e := <-w.ResultChan(): - require.Equal(t, et, e.Type) - require.Equal(t, o, e.Object) - return true - default: - return false - } + expectedLDCSpec := expectedLDCSpec(config) + + var actualLDC *placeholderv1alpha1.LoginDiscoveryConfig + for i := 0; i < 10; i++ { + actualLDC, err = client.PlaceholderV1alpha1(). + LoginDiscoveryConfigs(namespaceName). + Get(ctx, "placeholder-name-config", metav1.GetOptions{}) + if err == nil { + break } + time.Sleep(time.Millisecond * 750) } - require.Eventually( - t, - received(watch.Deleted, expectedLDC), - time.Second, - 3*time.Second, - ) - require.Eventually( - t, - received(watch.Added, expectedLDC), - time.Second, - 3*time.Second, - ) + require.NoError(t, err) + require.Equal(t, expectedLDCSpec, &actualLDC.Spec) } -func getExpectedLDC( - namespaceName string, - config *rest.Config, -) *placeholderv1alpha1.LoginDiscoveryConfig { - return &placeholderv1alpha1.LoginDiscoveryConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "placeholder-name-config", - Namespace: namespaceName, - }, - Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ - Server: config.Host, - CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), - }, +func expectedLDCSpec(config *rest.Config) *placeholderv1alpha1.LoginDiscoveryConfigSpec { + return &placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Server: "https://kind-control-plane:6443", //config.Host, // TODO FIX THIS + CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), } } diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 69abb0f5..5e7ca975 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -181,32 +181,41 @@ func TestGetAPIResourceList(t *testing.T) { actualResources := findResources(resourceGroupVersion, resources) require.NotNil(t, actualResources) - expectedResources := &metav1.APIResourceList{ - TypeMeta: metav1.TypeMeta{ - Kind: "APIResourceList", - APIVersion: "v1", - }, - GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", - APIResources: []metav1.APIResource{ - { - Name: "loginrequests", - Kind: "LoginRequest", - SingularName: "", // TODO(akeesler): what should this be? - Verbs: metav1.Verbs([]string{ - "create", - }), - }, - }, + expectedLoginRequestAPIResource := metav1.APIResource{ + Name: "loginrequests", + Kind: "LoginRequest", + SingularName: "", // TODO(akeesler): what should this be? + Verbs: metav1.Verbs([]string{ + "create", + }), + Namespaced: false, } - require.Equal(t, expectedResources, actualResources) -} -func TestGetAPIVersion(t *testing.T) { - client := library.NewPlaceholderNameClientset(t) + expectedLDCAPIResource := metav1.APIResource{ + Name: "logindiscoveryconfigs", + SingularName: "logindiscoveryconfig", + Namespaced: true, + Kind: "LoginDiscoveryConfig", + Verbs: metav1.Verbs([]string{ + "delete", "deletecollection", "get", "list", "patch", "create", "update", "watch", + }), + ShortNames: []string{"ldc"}, + StorageVersionHash: "unknown: to be filled in automatically below", + } - version, err := client.Discovery().ServerVersion() - require.NoError(t, err) - require.NotNil(t, version) // TODO(akeesler): what can we assert here? + expectedResourcesMap := map[string]metav1.APIResource{ + expectedLoginRequestAPIResource.Name: expectedLoginRequestAPIResource, + expectedLDCAPIResource.Name: expectedLDCAPIResource, + } + + require.Len(t, actualResources.APIResources, 2) + for _, actualAPIResource := range actualResources.APIResources { + if actualAPIResource.Name == expectedLDCAPIResource.Name { + // hard to predict the storage version hash (e.g. "t/+v41y+3e4=") so just don't worry about comparing them + expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash + } + require.Equal(t, expectedResourcesMap[actualAPIResource.Name], actualAPIResource) + } } func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup { From cf56c673294fee3e12b5a3498d6d5ff2c2885593 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 31 Jul 2020 17:22:12 -0700 Subject: [PATCH 12/17] Move LoginDiscoveryConfig to the crds.placeholder.suzerain-io.github.io group - Also includes bumping the api and client-go dependencies to the newer version which also moved LoginDiscoveryConfig to the crds.placeholder.suzerain-io.github.io group in the generated code --- deploy/crd.yaml | 4 +-- deploy/rbac.yaml | 2 +- go.mod | 4 +-- go.sum | 8 +++--- .../controller/logindiscovery/publisher.go | 18 ++++++------- .../logindiscovery/publisher_test.go | 24 ++++++++--------- internal/server/server.go | 2 +- test/integration/logindiscoveryconfig_test.go | 15 ++++++----- test/integration/loginrequest_test.go | 27 +++++++++---------- 9 files changed, 51 insertions(+), 53 deletions(-) diff --git a/deploy/crd.yaml b/deploy/crd.yaml index a859de09..2bf7f635 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -15,9 +15,9 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: logindiscoveryconfigs.placeholder.suzerain-io.github.io + name: logindiscoveryconfigs.crds.placeholder.suzerain-io.github.io spec: - group: placeholder.suzerain-io.github.io + group: crds.placeholder.suzerain-io.github.io versions: #! Any changes to these schemas should also be reflected in the types.go file(s) #! in https://github.com/suzerain-io/placeholder-name-api/tree/main/pkg/apis/placeholder diff --git a/deploy/rbac.yaml b/deploy/rbac.yaml index fd67592f..98caf1c3 100644 --- a/deploy/rbac.yaml +++ b/deploy/rbac.yaml @@ -38,7 +38,7 @@ rules: - apiGroups: [""] resources: [services] verbs: [create, get, list, patch, update, watch] - - apiGroups: [placeholder.suzerain-io.github.io] + - apiGroups: [crds.placeholder.suzerain-io.github.io] resources: [logindiscoveryconfigs] verbs: [create, get, list, update, watch] --- diff --git a/go.mod b/go.mod index f41972eb..299112eb 100644 --- a/go.mod +++ b/go.mod @@ -11,8 +11,8 @@ require ( github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f - github.com/suzerain-io/placeholder-name-api v0.0.0-20200731022217-d7e4c306f7fd - github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731022627-a9c34c8413ac + github.com/suzerain-io/placeholder-name-api v0.0.0-20200731224558-ff85679d3364 + github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731225637-b994efe19486 github.com/suzerain-io/placeholder-name/pkg/client v0.0.0-00010101000000-000000000000 k8s.io/api v0.19.0-rc.0 k8s.io/apimachinery v0.19.0-rc.0 diff --git a/go.sum b/go.sum index c62d86e6..1e580372 100644 --- a/go.sum +++ b/go.sum @@ -532,10 +532,10 @@ github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f h1:gZ6rAdl+VE9DT0yE52xY/kJZ/hOJYxwtsgGoPr5vItI= github.com/suzerain-io/controller-go v0.0.0-20200730212956-7f99b569ca9f/go.mod h1:+v9upryFWBJac6KXKlheGHr7e3kqpk1ldH1iIMFopMs= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200731022217-d7e4c306f7fd h1:rJT8jn+6g1Wy40nGOGULz9hQvuDq0MZSA+3JaTaqVgQ= -github.com/suzerain-io/placeholder-name-api v0.0.0-20200731022217-d7e4c306f7fd/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= -github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731022627-a9c34c8413ac h1:3QfNymuRno/01062Ns/xv70m8TxQjiJQKTYCjIztGJw= -github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731022627-a9c34c8413ac/go.mod h1:EFFPGm0xrUidCxQkF1g2pGdKifwo4U0Dwi70TTluNGM= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200731224558-ff85679d3364 h1:5NaQExCSh8+6YP3QNhWsWyMrEd+7zsnrznRlisydlZo= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200731224558-ff85679d3364/go.mod h1:OuYBJDpMMnvMUoBn+XeMWtHghuYk0cq9bNkNa3T8j/g= +github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731225637-b994efe19486 h1:JOPwCoRjsUu8E/E81BolafPfovnTmxGAgFnEV0R8T3U= +github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200731225637-b994efe19486/go.mod h1:0lk6jYt88I1632/5TIwpBPhQAewKuesNK+rKhfoegRk= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2 h1:Xr9gkxfOP0KQWXKNqmwe8vEeSUiUj4Rlee9CMVX2ZUQ= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM= github.com/tetafro/godot v0.4.2 h1:Dib7un+rYJFUi8vN0Bk6EHheKy6fv6ZzFURHw75g6m8= diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 0725437b..0d9547b5 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -17,9 +17,9 @@ import ( "k8s.io/klog/v2" "github.com/suzerain-io/controller-go" - placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + crdsplaceholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/crdsplaceholder/v1alpha1" placeholderclientset "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned" - placeholderv1alpha1informers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions/placeholder/v1alpha1" + crdsplaceholderv1alpha1informers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions/crdsplaceholder/v1alpha1" ) const ( @@ -54,14 +54,14 @@ type publisherController struct { namespace string placeholderClient placeholderclientset.Interface configMapInformer corev1informers.ConfigMapInformer - loginDiscoveryConfigInformer placeholderv1alpha1informers.LoginDiscoveryConfigInformer + loginDiscoveryConfigInformer crdsplaceholderv1alpha1informers.LoginDiscoveryConfigInformer } func NewPublisherController( namespace string, placeholderClient placeholderclientset.Interface, configMapInformer corev1informers.ConfigMapInformer, - loginDiscoveryConfigInformer placeholderv1alpha1informers.LoginDiscoveryConfigInformer, + loginDiscoveryConfigInformer crdsplaceholderv1alpha1informers.LoginDiscoveryConfigInformer, withInformer withInformerOptionFunc, ) controller.Controller { return controller.New( @@ -120,13 +120,13 @@ func (c *publisherController) Sync(ctx controller.Context) error { break } - discoveryConfig := placeholderv1alpha1.LoginDiscoveryConfig{ + discoveryConfig := crdsplaceholderv1alpha1.LoginDiscoveryConfig{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: configName, Namespace: c.namespace, }, - Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Spec: crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec{ Server: server, CertificateAuthorityData: certificateAuthorityData, }, @@ -140,7 +140,7 @@ func (c *publisherController) Sync(ctx controller.Context) error { func (c *publisherController) createOrUpdateLoginDiscoveryConfig( ctx context.Context, - discoveryConfig *placeholderv1alpha1.LoginDiscoveryConfig, + discoveryConfig *crdsplaceholderv1alpha1.LoginDiscoveryConfig, ) error { existingDiscoveryConfig, err := c.loginDiscoveryConfigInformer. Lister(). @@ -152,7 +152,7 @@ func (c *publisherController) createOrUpdateLoginDiscoveryConfig( } loginDiscoveryConfigs := c.placeholderClient. - PlaceholderV1alpha1(). + CrdsV1alpha1(). LoginDiscoveryConfigs(c.namespace) if notFound { if _, err := loginDiscoveryConfigs.Create( @@ -179,7 +179,7 @@ func (c *publisherController) createOrUpdateLoginDiscoveryConfig( return nil } -func equal(a, b *placeholderv1alpha1.LoginDiscoveryConfig) bool { +func equal(a, b *crdsplaceholderv1alpha1.LoginDiscoveryConfig) bool { return a.Spec.Server == b.Spec.Server && a.Spec.CertificateAuthorityData == b.Spec.CertificateAuthorityData } diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index 8d90354c..2299f0ee 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -24,7 +24,7 @@ import ( coretesting "k8s.io/client-go/testing" "github.com/suzerain-io/controller-go" - placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + crdsplaceholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/crdsplaceholder/v1alpha1" placeholderfake "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/clientset/versioned/fake" placeholderinformers "github.com/suzerain-io/placeholder-name-client-go/pkg/generated/informers/externalversions" ) @@ -60,7 +60,7 @@ func TestInformerFilters(t *testing.T) { r = require.New(t) observableWithInformerOption = NewObservableWithInformerOption() configMapInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().ConfigMaps() - loginDiscoveryConfigInformer := placeholderinformers.NewSharedInformerFactory(nil, 0).Placeholder().V1alpha1().LoginDiscoveryConfigs() + loginDiscoveryConfigInformer := placeholderinformers.NewSharedInformerFactory(nil, 0).Crds().V1alpha1().LoginDiscoveryConfigs() _ = NewPublisherController( installedInNamespace, nil, @@ -122,20 +122,20 @@ func TestInformerFilters(t *testing.T) { when("watching LoginDiscoveryConfig objects", func() { var subject controller.Filter - var target, wrongNamespace, wrongName, unrelated *placeholderv1alpha1.LoginDiscoveryConfig + var target, wrongNamespace, wrongName, unrelated *crdsplaceholderv1alpha1.LoginDiscoveryConfig it.Before(func() { subject = loginDiscoveryConfigInformerFilter - target = &placeholderv1alpha1.LoginDiscoveryConfig{ + target = &crdsplaceholderv1alpha1.LoginDiscoveryConfig{ ObjectMeta: metav1.ObjectMeta{Name: "placeholder-name-config", Namespace: installedInNamespace}, } - wrongNamespace = &placeholderv1alpha1.LoginDiscoveryConfig{ + wrongNamespace = &crdsplaceholderv1alpha1.LoginDiscoveryConfig{ ObjectMeta: metav1.ObjectMeta{Name: "placeholder-name-config", Namespace: "wrong-namespace"}, } - wrongName = &placeholderv1alpha1.LoginDiscoveryConfig{ + wrongName = &crdsplaceholderv1alpha1.LoginDiscoveryConfig{ ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: installedInNamespace}, } - unrelated = &placeholderv1alpha1.LoginDiscoveryConfig{ + unrelated = &crdsplaceholderv1alpha1.LoginDiscoveryConfig{ ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}, } }) @@ -194,18 +194,18 @@ func TestSync(t *testing.T) { var timeoutContextCancel context.CancelFunc var syncContext *controller.Context - var expectedLoginDiscoveryConfig = func(expectedNamespace, expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *placeholderv1alpha1.LoginDiscoveryConfig) { + var expectedLoginDiscoveryConfig = func(expectedNamespace, expectedServerURL, expectedCAData string) (schema.GroupVersionResource, *crdsplaceholderv1alpha1.LoginDiscoveryConfig) { expectedLoginDiscoveryConfigGVR := schema.GroupVersionResource{ - Group: placeholderv1alpha1.GroupName, + Group: crdsplaceholderv1alpha1.GroupName, Version: "v1alpha1", Resource: "logindiscoveryconfigs", } - expectedLoginDiscoveryConfig := &placeholderv1alpha1.LoginDiscoveryConfig{ + expectedLoginDiscoveryConfig := &crdsplaceholderv1alpha1.LoginDiscoveryConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "placeholder-name-config", Namespace: expectedNamespace, }, - Spec: placeholderv1alpha1.LoginDiscoveryConfigSpec{ + Spec: crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec{ Server: expectedServerURL, CertificateAuthorityData: expectedCAData, }, @@ -237,7 +237,7 @@ func TestSync(t *testing.T) { installedInNamespace, placeholderAPIClient, kubeInformers.Core().V1().ConfigMaps(), - placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), + placeholderInformers.Crds().V1alpha1().LoginDiscoveryConfigs(), controller.WithInformer, ) diff --git a/internal/server/server.go b/internal/server/server.go index a2dc9664..f4d07dfd 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -346,7 +346,7 @@ func wireControllerManagerRunFunc( serverInstallationNamespace, placeholder, k8sInformers.Core().V1().ConfigMaps(), - placeholderInformers.Placeholder().V1alpha1().LoginDiscoveryConfigs(), + placeholderInformers.Crds().V1alpha1().LoginDiscoveryConfigs(), controller.WithInformer, ), singletonWorker, diff --git a/test/integration/logindiscoveryconfig_test.go b/test/integration/logindiscoveryconfig_test.go index 8f37b5b4..132c6557 100644 --- a/test/integration/logindiscoveryconfig_test.go +++ b/test/integration/logindiscoveryconfig_test.go @@ -16,7 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" - placeholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" + crdsplaceholderv1alpha1 "github.com/suzerain-io/placeholder-name-api/pkg/apis/crdsplaceholder/v1alpha1" "github.com/suzerain-io/placeholder-name/test/library" ) @@ -32,7 +32,7 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { config := library.NewClientConfig(t) expectedLDCSpec := expectedLDCSpec(config) configList, err := client. - PlaceholderV1alpha1(). + CrdsV1alpha1(). LoginDiscoveryConfigs(namespaceName). List(ctx, metav1.ListOptions{}) require.NoError(t, err) @@ -50,7 +50,7 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { defer cancel() err := client. - PlaceholderV1alpha1(). + CrdsV1alpha1(). LoginDiscoveryConfigs(namespaceName). Delete(ctx, "placeholder-name-config", metav1.DeleteOptions{}) require.NoError(t, err) @@ -58,9 +58,10 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { config := library.NewClientConfig(t) expectedLDCSpec := expectedLDCSpec(config) - var actualLDC *placeholderv1alpha1.LoginDiscoveryConfig + var actualLDC *crdsplaceholderv1alpha1.LoginDiscoveryConfig for i := 0; i < 10; i++ { - actualLDC, err = client.PlaceholderV1alpha1(). + actualLDC, err = client. + CrdsV1alpha1(). LoginDiscoveryConfigs(namespaceName). Get(ctx, "placeholder-name-config", metav1.GetOptions{}) if err == nil { @@ -72,8 +73,8 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { require.Equal(t, expectedLDCSpec, &actualLDC.Spec) } -func expectedLDCSpec(config *rest.Config) *placeholderv1alpha1.LoginDiscoveryConfigSpec { - return &placeholderv1alpha1.LoginDiscoveryConfigSpec{ +func expectedLDCSpec(config *rest.Config) *crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec { + return &crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec{ Server: "https://kind-control-plane:6443", //config.Host, // TODO FIX THIS CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), } diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 5e7ca975..57ffaade 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -177,9 +177,10 @@ func TestGetAPIResourceList(t *testing.T) { } require.Equal(t, expectedGroup, actualGroup) - resourceGroupVersion := "placeholder.suzerain-io.github.io/v1alpha1" - actualResources := findResources(resourceGroupVersion, resources) - require.NotNil(t, actualResources) + actualPlaceHolderResources := findResources("placeholder.suzerain-io.github.io/v1alpha1", resources) + require.NotNil(t, actualPlaceHolderResources) + actualCrdsPlaceHolderResources := findResources("crds.placeholder.suzerain-io.github.io/v1alpha1", resources) + require.NotNil(t, actualPlaceHolderResources) expectedLoginRequestAPIResource := metav1.APIResource{ Name: "loginrequests", @@ -203,19 +204,15 @@ func TestGetAPIResourceList(t *testing.T) { StorageVersionHash: "unknown: to be filled in automatically below", } - expectedResourcesMap := map[string]metav1.APIResource{ - expectedLoginRequestAPIResource.Name: expectedLoginRequestAPIResource, - expectedLDCAPIResource.Name: expectedLDCAPIResource, - } + require.Len(t, actualPlaceHolderResources.APIResources, 1) + require.Equal(t, expectedLoginRequestAPIResource, actualPlaceHolderResources.APIResources[0]) - require.Len(t, actualResources.APIResources, 2) - for _, actualAPIResource := range actualResources.APIResources { - if actualAPIResource.Name == expectedLDCAPIResource.Name { - // hard to predict the storage version hash (e.g. "t/+v41y+3e4=") so just don't worry about comparing them - expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash - } - require.Equal(t, expectedResourcesMap[actualAPIResource.Name], actualAPIResource) - } + require.Len(t, actualCrdsPlaceHolderResources.APIResources, 1) + actualAPIResource := actualCrdsPlaceHolderResources.APIResources[0] + // workaround because its hard to predict the storage version hash (e.g. "t/+v41y+3e4=") + // so just don't worry about comparing that field + expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash + require.Equal(t, expectedLDCAPIResource, actualAPIResource) } func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup { From 548874a641a707d5524eab6a821dd0618ade7ead Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 31 Jul 2020 17:37:59 -0700 Subject: [PATCH 13/17] Move TestGetAPIResourceList to its own file Because it is now testing multiple api types --- test/integration/api_discovery_test.go | 96 ++++++++++++++++++++++++++ test/integration/loginrequest_test.go | 81 ---------------------- 2 files changed, 96 insertions(+), 81 deletions(-) create mode 100644 test/integration/api_discovery_test.go diff --git a/test/integration/api_discovery_test.go b/test/integration/api_discovery_test.go new file mode 100644 index 00000000..c4ca2303 --- /dev/null +++ b/test/integration/api_discovery_test.go @@ -0,0 +1,96 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package integration + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/suzerain-io/placeholder-name/test/library" +) + +func TestGetAPIResourceList(t *testing.T) { + client := library.NewPlaceholderNameClientset(t) + + groups, resources, err := client.Discovery().ServerGroupsAndResources() + require.NoError(t, err) + + groupName := "placeholder.suzerain-io.github.io" + actualGroup := findGroup(groupName, groups) + require.NotNil(t, actualGroup) + + expectedGroup := &metav1.APIGroup{ + Name: "placeholder.suzerain-io.github.io", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", + Version: "v1alpha1", + }, + }, + PreferredVersion: metav1.GroupVersionForDiscovery{ + GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", + Version: "v1alpha1", + }, + } + require.Equal(t, expectedGroup, actualGroup) + + actualPlaceHolderResources := findResources("placeholder.suzerain-io.github.io/v1alpha1", resources) + require.NotNil(t, actualPlaceHolderResources) + actualCrdsPlaceHolderResources := findResources("crds.placeholder.suzerain-io.github.io/v1alpha1", resources) + require.NotNil(t, actualPlaceHolderResources) + + expectedLoginRequestAPIResource := metav1.APIResource{ + Name: "loginrequests", + Kind: "LoginRequest", + SingularName: "", // TODO(akeesler): what should this be? + Verbs: metav1.Verbs([]string{ + "create", + }), + Namespaced: false, + } + + expectedLDCAPIResource := metav1.APIResource{ + Name: "logindiscoveryconfigs", + SingularName: "logindiscoveryconfig", + Namespaced: true, + Kind: "LoginDiscoveryConfig", + Verbs: metav1.Verbs([]string{ + "delete", "deletecollection", "get", "list", "patch", "create", "update", "watch", + }), + ShortNames: []string{"ldc"}, + StorageVersionHash: "unknown: to be filled in automatically below", + } + + require.Len(t, actualPlaceHolderResources.APIResources, 1) + require.Equal(t, expectedLoginRequestAPIResource, actualPlaceHolderResources.APIResources[0]) + + require.Len(t, actualCrdsPlaceHolderResources.APIResources, 1) + actualAPIResource := actualCrdsPlaceHolderResources.APIResources[0] + // workaround because its hard to predict the storage version hash (e.g. "t/+v41y+3e4=") + // so just don't worry about comparing that field + expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash + require.Equal(t, expectedLDCAPIResource, actualAPIResource) +} + +func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup { + for _, group := range groups { + if group.Name == name { + return group + } + } + return nil +} + +func findResources(groupVersion string, resources []*metav1.APIResourceList) *metav1.APIResourceList { + for _, resource := range resources { + if resource.GroupVersion == groupVersion { + return resource + } + } + return nil +} diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 57ffaade..38ec3537 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -151,84 +151,3 @@ func TestLoginRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T) { require.Empty(t, response.Spec) require.Nil(t, response.Status.Credential) } - -func TestGetAPIResourceList(t *testing.T) { - client := library.NewPlaceholderNameClientset(t) - - groups, resources, err := client.Discovery().ServerGroupsAndResources() - require.NoError(t, err) - - groupName := "placeholder.suzerain-io.github.io" - actualGroup := findGroup(groupName, groups) - require.NotNil(t, actualGroup) - - expectedGroup := &metav1.APIGroup{ - Name: "placeholder.suzerain-io.github.io", - Versions: []metav1.GroupVersionForDiscovery{ - { - GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", - Version: "v1alpha1", - }, - }, - PreferredVersion: metav1.GroupVersionForDiscovery{ - GroupVersion: "placeholder.suzerain-io.github.io/v1alpha1", - Version: "v1alpha1", - }, - } - require.Equal(t, expectedGroup, actualGroup) - - actualPlaceHolderResources := findResources("placeholder.suzerain-io.github.io/v1alpha1", resources) - require.NotNil(t, actualPlaceHolderResources) - actualCrdsPlaceHolderResources := findResources("crds.placeholder.suzerain-io.github.io/v1alpha1", resources) - require.NotNil(t, actualPlaceHolderResources) - - expectedLoginRequestAPIResource := metav1.APIResource{ - Name: "loginrequests", - Kind: "LoginRequest", - SingularName: "", // TODO(akeesler): what should this be? - Verbs: metav1.Verbs([]string{ - "create", - }), - Namespaced: false, - } - - expectedLDCAPIResource := metav1.APIResource{ - Name: "logindiscoveryconfigs", - SingularName: "logindiscoveryconfig", - Namespaced: true, - Kind: "LoginDiscoveryConfig", - Verbs: metav1.Verbs([]string{ - "delete", "deletecollection", "get", "list", "patch", "create", "update", "watch", - }), - ShortNames: []string{"ldc"}, - StorageVersionHash: "unknown: to be filled in automatically below", - } - - require.Len(t, actualPlaceHolderResources.APIResources, 1) - require.Equal(t, expectedLoginRequestAPIResource, actualPlaceHolderResources.APIResources[0]) - - require.Len(t, actualCrdsPlaceHolderResources.APIResources, 1) - actualAPIResource := actualCrdsPlaceHolderResources.APIResources[0] - // workaround because its hard to predict the storage version hash (e.g. "t/+v41y+3e4=") - // so just don't worry about comparing that field - expectedLDCAPIResource.StorageVersionHash = actualAPIResource.StorageVersionHash - require.Equal(t, expectedLDCAPIResource, actualAPIResource) -} - -func findGroup(name string, groups []*metav1.APIGroup) *metav1.APIGroup { - for _, group := range groups { - if group.Name == name { - return group - } - } - return nil -} - -func findResources(groupVersion string, resources []*metav1.APIResourceList) *metav1.APIResourceList { - for _, resource := range resources { - if resource.GroupVersion == groupVersion { - return resource - } - } - return nil -} From 597408a9774d1fbedfb68d4fe7b722ebd2e0ce8f Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Mon, 3 Aug 2020 10:17:11 -0400 Subject: [PATCH 14/17] Allow override of discovery URL via ConfigMap Signed-off-by: Andrew Keesler - Seems like the next step is to allow override of the CA bundle; I didn't do that here for simplicity of the commit, but seems like it is the right thing to do in the future. --- deploy/deployment.yaml | 2 + deploy/values.yaml | 2 + .../controller/logindiscovery/publisher.go | 7 ++ .../logindiscovery/publisher_test.go | 68 ++++++++++++++----- internal/server/server.go | 9 ++- pkg/config/api/types.go | 12 +++- pkg/config/config_test.go | 54 ++++++++++++--- pkg/config/testdata/happy.yaml | 2 + pkg/config/testdata/no-discovery.yaml | 5 ++ test/integration/client_test.go | 4 +- test/integration/deployment_test.go | 8 +-- test/integration/logindiscoveryconfig_test.go | 17 +++-- test/integration/loginrequest_test.go | 4 +- test/library/env.go | 22 ++++++ 14 files changed, 167 insertions(+), 49 deletions(-) create mode 100644 pkg/config/testdata/no-discovery.yaml create mode 100644 test/library/env.go diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml index 61d584e8..543b8db3 100644 --- a/deploy/deployment.yaml +++ b/deploy/deployment.yaml @@ -24,6 +24,8 @@ metadata: data: #@yaml/text-templated-strings placeholder-name.yaml: | + discovery: + url: (@= data.values.discovery_url @) webhook: url: (@= data.values.webhook_url @) caBundle: (@= data.values.webhook_ca_bundle @) diff --git a/deploy/values.yaml b/deploy/values.yaml index a5605efc..e63e611f 100644 --- a/deploy/values.yaml +++ b/deploy/values.yaml @@ -12,3 +12,5 @@ image_tag: #! e.g. latest webhook_url: #! e.g., https://example.com webhook_ca_bundle: #! e.g., LS0tLS1CRUdJTiBDRVJUSUZJQ0F... + +discovery_url: #! e.g., https://example.com diff --git a/internal/controller/logindiscovery/publisher.go b/internal/controller/logindiscovery/publisher.go index 0d9547b5..61186c26 100644 --- a/internal/controller/logindiscovery/publisher.go +++ b/internal/controller/logindiscovery/publisher.go @@ -52,6 +52,7 @@ type withInformerOptionFunc func( type publisherController struct { namespace string + serverOverride *string placeholderClient placeholderclientset.Interface configMapInformer corev1informers.ConfigMapInformer loginDiscoveryConfigInformer crdsplaceholderv1alpha1informers.LoginDiscoveryConfigInformer @@ -59,6 +60,7 @@ type publisherController struct { func NewPublisherController( namespace string, + serverOverride *string, placeholderClient placeholderclientset.Interface, configMapInformer corev1informers.ConfigMapInformer, loginDiscoveryConfigInformer crdsplaceholderv1alpha1informers.LoginDiscoveryConfigInformer, @@ -69,6 +71,7 @@ func NewPublisherController( Name: "publisher-controller", Syncer: &publisherController{ namespace: namespace, + serverOverride: serverOverride, placeholderClient: placeholderClient, configMapInformer: configMapInformer, loginDiscoveryConfigInformer: loginDiscoveryConfigInformer, @@ -120,6 +123,10 @@ func (c *publisherController) Sync(ctx controller.Context) error { break } + if c.serverOverride != nil { + server = *c.serverOverride + } + discoveryConfig := crdsplaceholderv1alpha1.LoginDiscoveryConfig{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/logindiscovery/publisher_test.go b/internal/controller/logindiscovery/publisher_test.go index 2299f0ee..8e3c69d9 100644 --- a/internal/controller/logindiscovery/publisher_test.go +++ b/internal/controller/logindiscovery/publisher_test.go @@ -64,6 +64,7 @@ func TestInformerFilters(t *testing.T) { _ = NewPublisherController( installedInNamespace, nil, + nil, configMapInformer, loginDiscoveryConfigInformer, observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters @@ -185,6 +186,7 @@ func TestSync(t *testing.T) { var r *require.Assertions var subject controller.Controller + var serverOverride *string var kubeInformerClient *kubernetesfake.Clientset var placeholderInformerClient *placeholderfake.Clientset var kubeInformers kubeinformers.SharedInformerFactory @@ -216,6 +218,26 @@ func TestSync(t *testing.T) { // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. var startInformersAndController = func() { + // Set this at the last second to allow for injection of server override. + subject = NewPublisherController( + installedInNamespace, + serverOverride, + placeholderAPIClient, + kubeInformers.Core().V1().ConfigMaps(), + placeholderInformers.Crds().V1alpha1().LoginDiscoveryConfigs(), + controller.WithInformer, + ) + + // Set this at the last second to support calling subject.Name(). + syncContext = &controller.Context{ + Context: timeoutContext, + Name: subject.Name(), + Key: controller.Key{ + Namespace: "kube-public", + Name: "cluster-info", + }, + } + // Must start informers before calling TestRunSynchronously() kubeInformers.Start(timeoutContext.Done()) placeholderInformers.Start(timeoutContext.Done()) @@ -232,23 +254,6 @@ func TestSync(t *testing.T) { placeholderAPIClient = placeholderfake.NewSimpleClientset() placeholderInformerClient = placeholderfake.NewSimpleClientset() placeholderInformers = placeholderinformers.NewSharedInformerFactory(placeholderInformerClient, 0) - - subject = NewPublisherController( - installedInNamespace, - placeholderAPIClient, - kubeInformers.Core().V1().ConfigMaps(), - placeholderInformers.Crds().V1alpha1().LoginDiscoveryConfigs(), - controller.WithInformer, - ) - - syncContext = &controller.Context{ - Context: timeoutContext, - Name: subject.Name(), - Key: controller.Key{ - Namespace: "kube-public", - Name: "cluster-info", - }, - } }) it.After(func() { @@ -321,6 +326,35 @@ func TestSync(t *testing.T) { r.EqualError(err, "could not create logindiscoveryconfig: create failed") }) }) + + when("a server override is passed to the controller", func() { + it("uses the server override field", func() { + serverOverride = new(string) + *serverOverride = "https://some-server-override" + + startInformersAndController() + err := controller.TestSync(t, subject, *syncContext) + r.NoError(err) + + expectedLoginDiscoveryConfigGVR, expectedLoginDiscoveryConfig := expectedLoginDiscoveryConfig( + installedInNamespace, + kubeServerURL, + caData, + ) + expectedLoginDiscoveryConfig.Spec.Server = "https://some-server-override" + + r.Equal( + []coretesting.Action{ + coretesting.NewCreateAction( + expectedLoginDiscoveryConfigGVR, + installedInNamespace, + expectedLoginDiscoveryConfig, + ), + }, + placeholderAPIClient.Actions(), + ) + }) + }) }) when("the LoginDiscoveryConfig already exists", func() { diff --git a/internal/server/server.go b/internal/server/server.go index f4d07dfd..d6164245 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -241,7 +241,12 @@ func (a *App) run( return fmt.Errorf("could not register API service: %w", err) } - cmrf := wireControllerManagerRunFunc(serverInstallationNamespace, k8sClient, placeholderClient) + cmrf := wireControllerManagerRunFunc( + serverInstallationNamespace, + cfg.DiscoveryConfig.URL, + k8sClient, + placeholderClient, + ) apiServerConfig, err := a.configServer( cert, webhookTokenAuthenticator, @@ -324,6 +329,7 @@ func createStaticCertKeyProvider(cert *tls.Certificate) (dynamiccertificates.Cer func wireControllerManagerRunFunc( serverInstallationNamespace string, + discoveryURLOverride *string, k8s kubernetes.Interface, placeholder placeholderclientset.Interface, ) func(ctx context.Context) { @@ -344,6 +350,7 @@ func wireControllerManagerRunFunc( WithController( logindiscovery.NewPublisherController( serverInstallationNamespace, + discoveryURLOverride, placeholder, k8sInformers.Core().V1().ConfigMaps(), placeholderInformers.Crds().V1alpha1().LoginDiscoveryConfigs(), diff --git a/pkg/config/api/types.go b/pkg/config/api/types.go index 795a2eb2..64cb25c4 100644 --- a/pkg/config/api/types.go +++ b/pkg/config/api/types.go @@ -7,7 +7,8 @@ package api // Config contains knobs to setup an instance of placeholder-name. type Config struct { - WebhookConfig WebhookConfigSpec `json:"webhook"` + WebhookConfig WebhookConfigSpec `json:"webhook"` + DiscoveryConfig DiscoveryConfigSpec `json:"discovery"` } // WebhookConfig contains configuration knobs specific to placeholder-name's use @@ -21,3 +22,12 @@ type WebhookConfigSpec struct { // to validate TLS connections to the WebhookURL. CABundle []byte `json:"caBundle"` } + +// DiscoveryConfigSpec contains configuration knobs specific to +// placeholder-name's publishing of discovery information. These values can be +// viewed as overrides, i.e., if these are set, then placeholder-name will +// publish these values in its discovery document instead of the ones it finds. +type DiscoveryConfigSpec struct { + // URL contains the URL at which placeholder-name can be contacted. + URL *string `json:"url,omitempty"` +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 52917469..82f06379 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -14,14 +14,50 @@ import ( ) func TestFromPath(t *testing.T) { - expect := require.New(t) - - config, err := FromPath("testdata/happy.yaml") - expect.NoError(err) - expect.Equal(config, &api.Config{ - WebhookConfig: api.WebhookConfigSpec{ - URL: "https://tuna.com/fish?marlin", - CABundle: []byte("-----BEGIN CERTIFICATE-----..."), + tests := []struct { + name string + path string + wantConfig *api.Config + }{ + { + name: "Happy", + path: "testdata/happy.yaml", + wantConfig: &api.Config{ + DiscoveryConfig: api.DiscoveryConfigSpec{ + URL: stringPtr("https://some.discovery/url"), + }, + WebhookConfig: api.WebhookConfigSpec{ + URL: "https://tuna.com/fish?marlin", + CABundle: []byte("-----BEGIN CERTIFICATE-----..."), + }, + }, }, - }) + { + name: "NoDiscovery", + path: "testdata/no-discovery.yaml", + wantConfig: &api.Config{ + DiscoveryConfig: api.DiscoveryConfigSpec{ + URL: nil, + }, + WebhookConfig: api.WebhookConfigSpec{ + URL: "https://tuna.com/fish?marlin", + CABundle: []byte("-----BEGIN CERTIFICATE-----..."), + }, + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + config, err := FromPath(test.path) + require.NoError(t, err) + require.Equal(t, test.wantConfig, config) + }) + } +} + +func stringPtr(s string) *string { + sPtr := new(string) + *sPtr = s + return sPtr } diff --git a/pkg/config/testdata/happy.yaml b/pkg/config/testdata/happy.yaml index 00c2e78b..ebbfac72 100644 --- a/pkg/config/testdata/happy.yaml +++ b/pkg/config/testdata/happy.yaml @@ -1,4 +1,6 @@ --- +discovery: + url: https://some.discovery/url webhook: url: https://tuna.com/fish?marlin caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tLi4u diff --git a/pkg/config/testdata/no-discovery.yaml b/pkg/config/testdata/no-discovery.yaml new file mode 100644 index 00000000..00c2e78b --- /dev/null +++ b/pkg/config/testdata/no-discovery.yaml @@ -0,0 +1,5 @@ +--- +webhook: + url: https://tuna.com/fish?marlin + caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tLi4u + diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 2b18f170..c34d3617 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -7,7 +7,6 @@ package integration import ( "context" - "os" "strings" "testing" "time" @@ -52,8 +51,7 @@ O2D8LtWhMbrYy755Fgq4H9s3vCgfvHY1AQ== ) func TestClient(t *testing.T) { - tmcClusterToken := os.Getenv("PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN") - require.NotEmptyf(t, tmcClusterToken, "must specify PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN env var for integration tests") + tmcClusterToken := library.Getenv(t, "PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() diff --git a/test/integration/deployment_test.go b/test/integration/deployment_test.go index 8b88a14b..b1c2de89 100644 --- a/test/integration/deployment_test.go +++ b/test/integration/deployment_test.go @@ -7,7 +7,6 @@ package integration import ( "context" - "os" "testing" "time" @@ -20,11 +19,8 @@ import ( ) func TestGetDeployment(t *testing.T) { - namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") - require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") - - deploymentName := os.Getenv("PLACEHOLDER_NAME_DEPLOYMENT") - require.NotEmptyf(t, deploymentName, "must specify PLACEHOLDER_NAME_DEPLOYMENT env var for integration tests") + namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE") + deploymentName := library.Getenv(t, "PLACEHOLDER_NAME_DEPLOYMENT") client := library.NewClientset(t) diff --git a/test/integration/logindiscoveryconfig_test.go b/test/integration/logindiscoveryconfig_test.go index 132c6557..bac62b17 100644 --- a/test/integration/logindiscoveryconfig_test.go +++ b/test/integration/logindiscoveryconfig_test.go @@ -8,7 +8,6 @@ package integration import ( "context" "encoding/base64" - "os" "testing" "time" @@ -21,8 +20,8 @@ import ( ) func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { - namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") - require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") + namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE") + discoveryURL := library.Getenv(t, "PLACEHOLDER_NAME_DISCOVERY_URL") client := library.NewPlaceholderNameClientset(t) @@ -30,7 +29,7 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { defer cancel() config := library.NewClientConfig(t) - expectedLDCSpec := expectedLDCSpec(config) + expectedLDCSpec := expectedLDCSpec(config, discoveryURL) configList, err := client. CrdsV1alpha1(). LoginDiscoveryConfigs(namespaceName). @@ -41,8 +40,8 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { } func TestReconcilingLoginDiscoveryConfig(t *testing.T) { - namespaceName := os.Getenv("PLACEHOLDER_NAME_NAMESPACE") - require.NotEmptyf(t, namespaceName, "must specify PLACEHOLDER_NAME_NAMESPACE env var for integration tests") + namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE") + discoveryURL := library.Getenv(t, "PLACEHOLDER_NAME_DISCOVERY_URL") client := library.NewPlaceholderNameClientset(t) @@ -56,7 +55,7 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { require.NoError(t, err) config := library.NewClientConfig(t) - expectedLDCSpec := expectedLDCSpec(config) + expectedLDCSpec := expectedLDCSpec(config, discoveryURL) var actualLDC *crdsplaceholderv1alpha1.LoginDiscoveryConfig for i := 0; i < 10; i++ { @@ -73,9 +72,9 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { require.Equal(t, expectedLDCSpec, &actualLDC.Spec) } -func expectedLDCSpec(config *rest.Config) *crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec { +func expectedLDCSpec(config *rest.Config, discoveryURL string) *crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec { return &crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec{ - Server: "https://kind-control-plane:6443", //config.Host, // TODO FIX THIS + Server: discoveryURL, CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), } } diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 38ec3537..55cc17bd 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -8,7 +8,6 @@ package integration import ( "context" "net/http" - "os" "testing" "time" @@ -37,8 +36,7 @@ func makeRequest(t *testing.T, spec v1alpha1.LoginRequestSpec) (*v1alpha1.LoginR } func TestSuccessfulLoginRequest(t *testing.T) { - tmcClusterToken := os.Getenv("PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN") - require.NotEmptyf(t, tmcClusterToken, "must specify PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN env var for integration tests") + tmcClusterToken := library.Getenv(t, "PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN") response, err := makeRequest(t, v1alpha1.LoginRequestSpec{ Type: v1alpha1.TokenLoginCredentialType, diff --git a/test/library/env.go b/test/library/env.go new file mode 100644 index 00000000..db0ff67a --- /dev/null +++ b/test/library/env.go @@ -0,0 +1,22 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package library + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +// Getenv gets the environment variable with key and asserts that it is not +// empty. It returns the value of the environment variable. +func Getenv(t *testing.T, key string) string { + t.Helper() + value := os.Getenv(key) + require.NotEmptyf(t, value, "must specify %s env var for integration tests", key) + return value +} From e884cef1ef435f2716a9f5fd77dbee63bbe2f6da Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Mon, 3 Aug 2020 10:29:54 -0400 Subject: [PATCH 15/17] Resolve SingularName TODO with comment Signed-off-by: Andrew Keesler --- test/integration/api_discovery_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/integration/api_discovery_test.go b/test/integration/api_discovery_test.go index c4ca2303..d35b7abe 100644 --- a/test/integration/api_discovery_test.go +++ b/test/integration/api_discovery_test.go @@ -45,13 +45,17 @@ func TestGetAPIResourceList(t *testing.T) { require.NotNil(t, actualPlaceHolderResources) expectedLoginRequestAPIResource := metav1.APIResource{ - Name: "loginrequests", - Kind: "LoginRequest", - SingularName: "", // TODO(akeesler): what should this be? + Name: "loginrequests", + Kind: "LoginRequest", Verbs: metav1.Verbs([]string{ "create", }), Namespaced: false, + + // This is currently an empty string in the response; maybe it should not be + // empty? Seems like no harm in keeping it like this for now, but feel free + // to update in the future if there is a compelling reason to do so. + SingularName: "", } expectedLDCAPIResource := metav1.APIResource{ From ca80d87dcfe34fa96dc9154c037e2e8846b713eb Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 3 Aug 2020 14:36:08 -0400 Subject: [PATCH 16/17] Use rest.Config for discovery URL instead of env var - Why? Because the discovery URL is already there in the kubeconfig; let's not make our lives more complicated by passing it in via an env var. - Also allow for ytt callers to not specify data.values.discovery_url - there are going to be a non-trivial number of installers of placeholder-name that want to use the server URL found in the cluster-info ConfigMap. Signed-off-by: Andrew Keesler --- deploy/deployment.yaml | 2 +- test/integration/logindiscoveryconfig_test.go | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/deploy/deployment.yaml b/deploy/deployment.yaml index 543b8db3..35024701 100644 --- a/deploy/deployment.yaml +++ b/deploy/deployment.yaml @@ -25,7 +25,7 @@ data: #@yaml/text-templated-strings placeholder-name.yaml: | discovery: - url: (@= data.values.discovery_url @) + url: (@= data.values.discovery_url or "null" @) webhook: url: (@= data.values.webhook_url @) caBundle: (@= data.values.webhook_ca_bundle @) diff --git a/test/integration/logindiscoveryconfig_test.go b/test/integration/logindiscoveryconfig_test.go index bac62b17..5a5cc169 100644 --- a/test/integration/logindiscoveryconfig_test.go +++ b/test/integration/logindiscoveryconfig_test.go @@ -21,7 +21,6 @@ import ( func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE") - discoveryURL := library.Getenv(t, "PLACEHOLDER_NAME_DISCOVERY_URL") client := library.NewPlaceholderNameClientset(t) @@ -29,7 +28,7 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { defer cancel() config := library.NewClientConfig(t) - expectedLDCSpec := expectedLDCSpec(config, discoveryURL) + expectedLDCSpec := expectedLDCSpec(config) configList, err := client. CrdsV1alpha1(). LoginDiscoveryConfigs(namespaceName). @@ -41,7 +40,6 @@ func TestSuccessfulLoginDiscoveryConfig(t *testing.T) { func TestReconcilingLoginDiscoveryConfig(t *testing.T) { namespaceName := library.Getenv(t, "PLACEHOLDER_NAME_NAMESPACE") - discoveryURL := library.Getenv(t, "PLACEHOLDER_NAME_DISCOVERY_URL") client := library.NewPlaceholderNameClientset(t) @@ -55,7 +53,7 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { require.NoError(t, err) config := library.NewClientConfig(t) - expectedLDCSpec := expectedLDCSpec(config, discoveryURL) + expectedLDCSpec := expectedLDCSpec(config) var actualLDC *crdsplaceholderv1alpha1.LoginDiscoveryConfig for i := 0; i < 10; i++ { @@ -72,9 +70,9 @@ func TestReconcilingLoginDiscoveryConfig(t *testing.T) { require.Equal(t, expectedLDCSpec, &actualLDC.Spec) } -func expectedLDCSpec(config *rest.Config, discoveryURL string) *crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec { +func expectedLDCSpec(config *rest.Config) *crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec { return &crdsplaceholderv1alpha1.LoginDiscoveryConfigSpec{ - Server: discoveryURL, + Server: config.Host, CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), } } From 12120d7e8bb6280e2985e2d76e045a18b3561f70 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Mon, 3 Aug 2020 14:45:39 -0400 Subject: [PATCH 17/17] Force CI to run.