Skip to content
This repository has been archived by the owner on Jul 4, 2024. It is now read-only.

Commit

Permalink
Address operation controller review feedback (#1762)
Browse files Browse the repository at this point in the history 
* Address operation controller review feedback

* Update components versions

* Update go.mod and go.sum files

* Add more directive tests

* Address part of the review comments

* Extract operations endpoint into configuration

* Fix rebase/merge problem

* downgrade k8s dependencies

* Adapt application converters and e2e tests

* Fix converters

* Remove internal address variables

* Downgrade connector's k8s deps

* Downgrade director's k8s deps

* Fix connector tests

* Fix operation-contoller tests

* Update connector component version

* Bump schema migrator

Co-authored-by: NickyMateev <[email protected]>
  • Loading branch information
@PetarTodorovv @NickyMateev
PetarTodorovv and NickyMateev authored Mar 12, 2021
1 parent 4c52cc0 commit 782ba84
Show file tree
Hide file tree
Showing 50 changed files with 899 additions and 817 deletions.
4 changes: 4 additions & 0 deletions chart/compass/charts/director/templates/deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ spec:
value: {{ .Values.deployment.args.token.csrExpiration | quote }}
- name: APP_URL
value: "https://{{ .Values.global.gateway.tls.host }}.{{ .Values.global.ingress.domainName }}{{ .Values.global.director.prefix }}"
- name: APP_OPERATION_PATH
value: {{ .Values.global.director.operations.path }}
- name: APP_LAST_OPERATION_PATH
value: {{ .Values.global.director.operations.lastOperationPath }}
- name: APP_CONNECTOR_URL
value: "https://{{ .Values.global.gateway.tls.host }}.{{ .Values.global.ingress.domainName }}{{ .Values.global.connector.prefix }}/graphql"
- name: APP_CONFIGURATION_FILE
Expand Down
4 changes: 2 additions & 2 deletions chart/compass/charts/operations-controller/templates/deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ spec:
value: "true"
- name: GRAPHQL_CLIENT_GRAPHQL_ENDPOINT
value: "http://{{ .Values.global.gateway.tls.host }}.{{ .Release.Namespace }}.svc.cluster.local:{{.Values.global.gateway.port }}/director/graphql"
- name: DIRECTOR_INTERNAL_ADDRESS
value: "http://compass-director.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.global.director.operations.port }}"
- name: DIRECTOR_OPERATION_ENDPOINT
value: "http://compass-director.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.global.director.operations.port }}{{ .Values.global.director.operations.path }}"
image: {{ .Values.global.images.containerRegistry.path }}/{{ .Values.global.images.connector.dir }}compass-operations-controller:{{ .Values.global.images.operations_controller.version }}
name: manager
ports:
Expand Down
12 changes: 7 additions & 5 deletions chart/compass/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ global:
path: eu.gcr.io/kyma-project/incubator
connector:
dir:
version: "PR-1741"
version: "PR-1762"
connectivity_adapter:
dir:
version: "PR-1750"
Expand All @@ -22,13 +22,13 @@ global:
version: "PR-1750"
director:
dir:
version: "PR-1741"
version: "PR-1762"
gateway:
dir:
version: "PR-1750"
operations_controller:
dir:
version: "PR-1750"
version: "PR-1762"
tenant_fetcher:
dir:
version: "PR-1750"
Expand All @@ -37,7 +37,7 @@ global:
version: "PR-15"
schema_migrator:
dir:
version: "PR-1761"
version: "PR-1762"
system_broker:
dir:
version: "PR-1750"
Expand All @@ -55,7 +55,7 @@ global:
tests:
director:
dir:
version: "PR-1741"
version: "PR-1762"
connector:
dir:
version: "PR-1741"
Expand Down Expand Up @@ -101,6 +101,8 @@ global:
port: 3003
operations:
port: 3002
path: "/operation"
lastOperationPath: "/last_operation"

clientIDHeaderKey: client_user

Expand Down
6 changes: 3 additions & 3 deletions components/connector/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/vektah/gqlparser v1.3.1
github.com/vrischmann/envconfig v1.3.0
golang.org/x/text v0.3.5 // indirect
k8s.io/api v0.20.2
k8s.io/apimachinery v0.20.2
k8s.io/client-go v0.20.2
k8s.io/api v0.17.2
k8s.io/apimachinery v0.17.2
k8s.io/client-go v0.17.2
)
8 changes: 8 additions & 0 deletions components/connector/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/evanphx/json-patch v4.2.0+incompatible h1:fUDGZCv/7iAN7u0puUVhvKCcsR6vRfwrJatElLBEf0I=
github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch v4.9.0+incompatible h1:kLcOMZeuLAJvL2BPWLMIj5oaZQobrkAqrL+WFZwQses=
github.com/evanphx/json-patch v4.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
Expand Down Expand Up @@ -250,6 +251,7 @@ github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs=
github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d h1:7XGaL1e6bYS1yIonGp9761ExpPPV1ui0SAC59Yube9k=
github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY=
github.com/googleapis/gnostic v0.4.1 h1:DLJCy1n/vrD4HPjOvYcT8aYQXpPIzoRZONaYwyycI+I=
github.com/googleapis/gnostic v0.4.1/go.mod h1:LRhVm6pbyptWbWbuZ38d1eyptfvIytN3ir6b65WBswg=
Expand Down Expand Up @@ -901,12 +903,15 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
k8s.io/api v0.17.2 h1:NF1UFXcKN7/OOv1uxdRz3qfra8AHsPav5M93hlV9+Dc=
k8s.io/api v0.17.2/go.mod h1:BS9fjjLc4CMuqfSO8vgbHPKMt5+SF0ET6u/RVDihTo4=
k8s.io/api v0.20.2 h1:y/HR22XDZY3pniu9hIFDLpUCPq2w5eQ6aV/VFQ7uJMw=
k8s.io/api v0.20.2/go.mod h1:d7n6Ehyzx+S+cE3VhTGfVNNqtGc/oL9DCdYYahlurV8=
k8s.io/apimachinery v0.17.2 h1:hwDQQFbdRlpnnsR64Asdi55GyCaIP/3WQpMmbNBeWr4=
k8s.io/apimachinery v0.17.2/go.mod h1:b9qmWdKlLuU9EBh+06BtLcSf/Mu89rWL33naRxs1uZg=
k8s.io/apimachinery v0.20.2 h1:hFx6Sbt1oG0n6DZ+g4bFt5f6BoMkOjKWsQFu077M3Vg=
k8s.io/apimachinery v0.20.2/go.mod h1:WlLqWAHZGg07AeltaI0MV5uk1Omp8xaN0JGLY6gkRpU=
k8s.io/client-go v0.17.2 h1:ndIfkfXEGrNhLIgkr0+qhRguSD3u6DCmonepn1O6NYc=
k8s.io/client-go v0.17.2/go.mod h1:QAzRgsa0C2xl4/eVpeVAZMvikCn8Nm81yqVx3Kk9XYI=
k8s.io/client-go v0.20.2 h1:uuf+iIAbfnCSw8IGAv/Rg0giM+2bOzHLOsbbrwrdhNQ=
k8s.io/client-go v0.20.2/go.mod h1:kH5brqWqp7HDxUFKoEgiI4v8G1xzbe9giaCenUWJzgE=
Expand All @@ -919,9 +924,11 @@ k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
k8s.io/klog/v2 v2.4.0 h1:7+X0fUguPyrKEC4WjH8iGDg3laWgMo5tMnRTIGTTxGQ=
k8s.io/klog/v2 v2.4.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a h1:UcxjrRMyNx/i/y8G7kPvLyy7rfbeuf1PYyBf973pgyU=
k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E=
k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd h1:sOHNzJIkytDF6qadMNKhhDRpc6ODik8lVC6nOur7B2c=
k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAGcJo0Tvi+dK12EcqSLqcWsryKMpfM=
k8s.io/utils v0.0.0-20191114184206-e782cd3c129f h1:GiPwtSzdP43eI1hpPCbROQCCIgCuiMMNF8YUVLF3vJo=
k8s.io/utils v0.0.0-20191114184206-e782cd3c129f/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
k8s.io/utils v0.0.0-20201110183641-67b214c5f920 h1:CbnUZsM497iRC5QMVkHwyl8s2tB3g7yaSHkYPkpgelw=
k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
Expand All @@ -932,6 +939,7 @@ sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e h1:4Z09Hglb
sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=
sigs.k8s.io/structured-merge-diff/v4 v4.0.2 h1:YHQV7Dajm86OuqnIR6zAelnDWBRjo+YhYV9PmGrh1s8=
sigs.k8s.io/structured-merge-diff/v4 v4.0.2/go.mod h1:bJZC9H9iH24zzfZ/41RGcq60oK1F7G282QMXDPYydCw=
sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs=
sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
Expand Down
2 changes: 1 addition & 1 deletion components/connector/internal/certificates/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (cl *certLoader) Run(ctx context.Context) {
}

func (cl *certLoader) loadSecretToCache(ctx context.Context, secret types.NamespacedName) {
secretData, appError := cl.secretsRepository.Get(ctx, secret)
secretData, appError := cl.secretsRepository.Get(secret)

if appError != nil {
log.C(ctx).WithError(appError).Errorf("Failed to load secret %s to cache", secret.String())
Expand Down
2 changes: 1 addition & 1 deletion components/connector/internal/revocation/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (rl *revokedCertificatesLoader) startKubeWatch(ctx context.Context) {
default:
}
log.C(ctx).Info("Starting watcher for revocation list configmap changes...")
watcher, err := rl.configMapManager.Watch(ctx, metav1.ListOptions{
watcher, err := rl.configMapManager.Watch(metav1.ListOptions{
FieldSelector: "metadata.name=" + rl.configMapName,
Watch: true,
})
Expand Down
4 changes: 2 additions & 2 deletions components/connector/internal/revocation/loader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func prep(ctx context.Context, number int) (Cache, *testWatch, *mocks.Manager) {
}
configListManagerMock := &mocks.Manager{}
configListManagerMock.
On("Watch", mock.Anything, mock.AnythingOfType("v1.ListOptions")).
On("Watch", mock.AnythingOfType("v1.ListOptions")).
Return(watcher, nil).
Times(number)
loader := NewRevokedCertificatesLoader(cache, configListManagerMock, configMapName, time.Millisecond)
Expand Down Expand Up @@ -192,7 +192,7 @@ func Test_revokedCertificatesLoader(t *testing.T) {
newWatcher := &testWatch{
events: make(chan watch.Event, 100),
}
managerMock.On("Watch", mock.Anything, mock.AnythingOfType("v1.ListOptions")).
managerMock.On("Watch", mock.AnythingOfType("v1.ListOptions")).
Return(newWatcher, nil).Once()

newWatcher.putEvent(watch.Event{
Expand Down
49 changes: 23 additions & 26 deletions components/connector/internal/revocation/mocks/Manager.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions components/connector/internal/revocation/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (

//go:generate mockery -name=Manager
type Manager interface {
Get(ctx context.Context, name string, options metav1.GetOptions) (*v1.ConfigMap, error)
Update(ctx context.Context, configmap *v1.ConfigMap, options metav1.UpdateOptions) (*v1.ConfigMap, error)
Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error)
Get(name string, options metav1.GetOptions) (*v1.ConfigMap, error)
Update(configmap *v1.ConfigMap) (*v1.ConfigMap, error)
Watch(opts metav1.ListOptions) (watch.Interface, error)
}

//go:generate mockery --name=RevokedCertificatesRepository
Expand All @@ -37,7 +37,7 @@ func NewRepository(configMapManager Manager, configMapName string, revokedCertsC
}

func (r *revokedCertifiatesRepository) Insert(ctx context.Context, hash string) error {
configMap, err := r.configMapManager.Get(ctx, r.configMapName, metav1.GetOptions{})
configMap, err := r.configMapManager.Get(r.configMapName, metav1.GetOptions{})
if err != nil {
return err
}
Expand All @@ -52,7 +52,7 @@ func (r *revokedCertifiatesRepository) Insert(ctx context.Context, hash string)
updatedConfigMap.Data = revokedCerts

err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
_, err = r.configMapManager.Update(ctx, updatedConfigMap, metav1.UpdateOptions{})
_, err = r.configMapManager.Update(updatedConfigMap)
return err
})

Expand Down
12 changes: 6 additions & 6 deletions components/connector/internal/revocation/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ func TestRevokedCertificatesRepository(t *testing.T) {
someHash := "someHash"
configListManagerMock := &mocks.Manager{}

configListManagerMock.On("Get", ctx, configMapName, mock.AnythingOfType("v1.GetOptions")).Return(
configListManagerMock.On("Get", configMapName, mock.AnythingOfType("v1.GetOptions")).Return(
&v1.ConfigMap{
Data: nil,
}, nil)

configListManagerMock.On("Update", ctx, &v1.ConfigMap{
configListManagerMock.On("Update", &v1.ConfigMap{
Data: map[string]string{
someHash: someHash,
}}, mock.AnythingOfType("v1.UpdateOptions")).Return(&v1.ConfigMap{
}}).Return(&v1.ConfigMap{
Data: map[string]string{
someHash: someHash,
}}, nil)
Expand All @@ -90,15 +90,15 @@ func TestRevokedCertificatesRepository(t *testing.T) {
someHash := "someHash"
configListManagerMock := &mocks.Manager{}

configListManagerMock.On("Get", ctx, configMapName, mock.AnythingOfType("v1.GetOptions")).Return(
configListManagerMock.On("Get", configMapName, mock.AnythingOfType("v1.GetOptions")).Return(
&v1.ConfigMap{
Data: nil,
}, nil)

configListManagerMock.On("Update", ctx, &v1.ConfigMap{
configListManagerMock.On("Update", &v1.ConfigMap{
Data: map[string]string{
someHash: someHash,
}}, mock.AnythingOfType("v1.UpdateOptions")).Return(nil, errors.New("some error"))
}}).Return(nil, errors.New("some error"))

repository := NewRepository(configListManagerMock, configMapName, cache)

Expand Down
21 changes: 9 additions & 12 deletions components/connector/internal/secrets/mocks/Manager.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 782ba84

Please sign in to comment.