feat: implements kms v2beta1 APIs #210

nilekhc · 2023-04-11T18:26:54Z

Reason for Change:

This PR adds KMS V2 Beta1 APIs to the plugin

Issue Fixed:

fixes #167

Notes for Reviewers:

pkg/plugin/v2_server.go

nilekhc · 2023-04-11T18:30:47Z

cc @enj

cmd/server/main.go

go.mod

pkg/plugin/v2_server.go

cmd/server/main.go

pkg/plugin/v2_server.go

cmd/server/main.go

pkg/plugin/v2_server.go

enj · 2023-04-12T21:43:21Z

Thinking out loud: should we, at least for the time being, append the current UTC date to the KMS key ID? This would cause a daily DEK rotation on the API server (and thus limit how long a DEK is used for, which is one of the open concerns we have around this API).

cmd/server/main.go

nilekhc · 2023-04-18T00:39:24Z

@aramase @enj @ritazh This is ready for review. PTAL when you get a chance.

.pipelines/templates/e2e-kind-template.yml

Makefile

sozercan · 2023-04-18T18:45:57Z

Makefile

+e2e-kmsv2-setup-kind: setup-local-registry
+	./scripts/setup-kmsv2-kind-cluster.sh &
+	./scripts/connect-registry.sh &
+	sleep 90s


is it possible to watch/poll something instead of hardcoded sleep?

Tried with the wait but it didn't work. Though we do have a check in the CI for cluster health: https://github.com/Azure/kubernetes-kms/blob/master/.pipelines/templates/cluster-health-template.yml

can we run both async and wait for pids to finish? similar to https://github.com/open-policy-agent/gatekeeper/blob/master/.github/workflows/release.yaml#L110-L120

When we run the process in the background ADO marks the task as completed

The error is:

The STDIO streams did not close within 10 seconds of the exit event from process '/bin/bash'. This may indicate a child process inherited the STDIO streams and has not yet exited.

I think this is the reason we have sleep, to hold on to the task for enough time so that the registry can be connected to the Kind cluster and created successfully.

@aramase to add any additional context.

pkg/plugin/kms_v2_server.go

pkg/plugin/kms_v2_server_test.go

nilekhc · 2023-04-18T20:59:18Z

Thanks for the review @sozercan. I have addressed your comments. PTAL when you get a chance.

.pipelines/templates/e2e-kind-template.yml

pkg/version/version.go

scripts/setup-kmsv2-kind-cluster.sh

tests/e2e/kmsv2-encryption-config.yaml

tests/e2e/testkmsv2.bats

go.mod

pkg/plugin/keyvault.go

tests/e2e/testkmsv2.bats

pkg/plugin/keyvault.go

enj

Minor stuff, this LGTM. @aramase PTAL at the metrics stuff.

docs/metrics.md

cmd/server/main.go

pkg/plugin/kms_v2_server_test.go

docs/metrics.md

pkg/metrics/stats_reporter.go

pkg/plugin/healthz.go

pkg/plugin/healthz_test.go

pkg/plugin/keyvault.go

pkg/plugin/kms_v2_server.go

tests/client/client_test.go

enj · 2023-05-15T14:58:53Z

btw this line in the README will not be correct once this PR merges:

Check that the stored secret is prefixed with k8s:enc:kms:v1:azurekmsprovider. This indicates the Azure KMS provider has encrypted the data.

nilekhc · 2023-05-15T21:23:50Z

btw this line in the README will not be correct once this PR merges:

Check that the stored secret is prefixed with k8s:enc:kms:v1:azurekmsprovider. This indicates the Azure KMS provider has encrypted the data.

@enj, I have made an update to this.

aramase

Just few small nits, but overall lgtm!

README.md

pkg/metrics/stats_reporter.go

pkg/plugin/keyvault.go

Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>

aramase

LGTM

nilekhc commented Apr 11, 2023

View reviewed changes

pkg/plugin/v2_server.go Outdated Show resolved Hide resolved

enj requested changes Apr 11, 2023

View reviewed changes

nilekhc mentioned this pull request Apr 11, 2023

chore: updates golangci-lint to v1.52.2 #212

Merged

aramase requested changes Apr 11, 2023

View reviewed changes

cmd/server/main.go Outdated Show resolved Hide resolved

pkg/plugin/v2_server.go Outdated Show resolved Hide resolved

nilekhc force-pushed the kms-v2beta1 branch from fd09f85 to 3d32e8b Compare April 14, 2023 23:22

enj reviewed Apr 14, 2023

View reviewed changes

cmd/server/main.go Outdated Show resolved Hide resolved

nilekhc force-pushed the kms-v2beta1 branch 8 times, most recently from 590d38a to 733647c Compare April 18, 2023 00:32

nilekhc marked this pull request as ready for review April 18, 2023 00:38

sozercan reviewed Apr 18, 2023

View reviewed changes

.pipelines/templates/e2e-kind-template.yml Outdated Show resolved Hide resolved

sozercan reviewed Apr 18, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

sozercan reviewed Apr 18, 2023

View reviewed changes

pkg/plugin/kms_v2_server.go Outdated Show resolved Hide resolved

sozercan reviewed Apr 18, 2023

View reviewed changes

pkg/plugin/kms_v2_server_test.go Outdated Show resolved Hide resolved

nilekhc force-pushed the kms-v2beta1 branch 2 times, most recently from b319d0d to 42019af Compare April 18, 2023 20:51

aramase requested changes Apr 18, 2023

View reviewed changes

nilekhc force-pushed the kms-v2beta1 branch 3 times, most recently from 18453b6 to 52090e3 Compare April 20, 2023 00:12

nilekhc force-pushed the kms-v2beta1 branch 3 times, most recently from bdc25f7 to a209f44 Compare May 8, 2023 22:14

nilekhc requested review from enj and aramase May 8, 2023 22:19

enj reviewed May 9, 2023

View reviewed changes

nilekhc mentioned this pull request May 11, 2023

Use encryption algorithm received in encryptResponse for decrypt request #234

Open

nilekhc force-pushed the kms-v2beta1 branch from 68da082 to 4eaef03 Compare May 11, 2023 22:11

nilekhc requested a review from enj May 12, 2023 00:18

enj previously approved these changes May 12, 2023

View reviewed changes

docs/metrics.md Outdated Show resolved Hide resolved

cmd/server/main.go Show resolved Hide resolved

pkg/plugin/kms_v2_server_test.go Outdated Show resolved Hide resolved

pkg/plugin/kms_v2_server_test.go Outdated Show resolved Hide resolved

aramase requested changes May 12, 2023

View reviewed changes

nilekhc mentioned this pull request May 15, 2023

Move logging to mlog #236

Closed

nilekhc dismissed enj’s stale review via 64dc67b May 15, 2023 20:58

nilekhc force-pushed the kms-v2beta1 branch 2 times, most recently from 64dc67b to 247278e Compare May 15, 2023 21:22

nilekhc requested review from aramase and enj May 15, 2023 21:24

nilekhc force-pushed the kms-v2beta1 branch 2 times, most recently from 8ed0346 to f6a087f Compare May 15, 2023 21:38

nilekhc mentioned this pull request May 15, 2023

feat: Integration tests for KMS v2 #238

Merged

nilekhc force-pushed the kms-v2beta1 branch from f6a087f to a029a77 Compare May 15, 2023 22:49

aramase reviewed May 17, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

pkg/metrics/stats_reporter.go Outdated Show resolved Hide resolved

pkg/plugin/keyvault.go Outdated Show resolved Hide resolved

pkg/plugin/keyvault.go Outdated Show resolved Hide resolved

pkg/plugin/keyvault.go Outdated Show resolved Hide resolved

feat: implements kms v2beta1 APIs

955274e

Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>

nilekhc force-pushed the kms-v2beta1 branch from a029a77 to 955274e Compare May 17, 2023 18:12

aramase approved these changes May 17, 2023

View reviewed changes

enj approved these changes May 17, 2023

View reviewed changes

nilekhc merged commit 71ea631 into Azure:master May 17, 2023

nilekhc deleted the kms-v2beta1 branch May 17, 2023 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implements kms v2beta1 APIs #210

feat: implements kms v2beta1 APIs #210

nilekhc commented Apr 11, 2023 •

edited by aramase

Loading

nilekhc commented Apr 11, 2023

enj commented Apr 12, 2023

nilekhc commented Apr 18, 2023

sozercan Apr 18, 2023

nilekhc Apr 18, 2023

sozercan Apr 25, 2023 •

edited

Loading

nilekhc Apr 25, 2023

nilekhc commented Apr 18, 2023

enj left a comment

enj commented May 15, 2023

nilekhc commented May 15, 2023

aramase left a comment

aramase left a comment

feat: implements kms v2beta1 APIs #210

feat: implements kms v2beta1 APIs #210

Conversation

nilekhc commented Apr 11, 2023 • edited by aramase Loading

nilekhc commented Apr 11, 2023

enj commented Apr 12, 2023

nilekhc commented Apr 18, 2023

sozercan Apr 18, 2023

Choose a reason for hiding this comment

nilekhc Apr 18, 2023

Choose a reason for hiding this comment

sozercan Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

nilekhc Apr 25, 2023

Choose a reason for hiding this comment

nilekhc commented Apr 18, 2023

enj left a comment

Choose a reason for hiding this comment

enj commented May 15, 2023

nilekhc commented May 15, 2023

aramase left a comment

Choose a reason for hiding this comment

aramase left a comment

Choose a reason for hiding this comment

nilekhc commented Apr 11, 2023 •

edited by aramase

Loading

sozercan Apr 25, 2023 •

edited

Loading