feat(VPCEndpointService): add custom resource and `allowedPrincipals` functionality by marcdavoli · Pull Request #172 · aws-controllers-k8s/ec2-controller

marcdavoli · 2023-12-21T18:42:31Z

Related to:

Add VPCEndpointService resource to ec2 controller community#1711

Description of changes:
This is the continuation of PR #134 (Credit to @ib-ak !).
This PR:

Adds the VPCEndpointService custom resource from the code-generator
Adds the field spec.allowedPrincipals of type []string, which allows the user to specify a list of ARNs to share the VPC Endpoint Service with. Any Principals added to this list will have the VPC Endpoint Service shared with them. This field also ensures that Principals that are removed from the list have the VPC Endpoint Service share removed.
Tests are added for creation/deletion and for adding/deleting Principals to the Allowed List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow · 2023-12-21T18:42:42Z

Hi @marcdavoli. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

a-hilaly · 2023-12-21T18:43:15Z

/ok-to-test

…vice)/add-custom-resource

…ns operation

a-hilaly

Thank you, very much! i have few comments in-line

a-hilaly · 2023-12-29T21:14:23Z

apis/v1alpha1/ack-generate-metadata.yaml

  go_version: go1.21.5
-  version: v0.28.0
-api_directory_checksum: d452bf19bfd1496aacdc215bf7cc9ea86c55c122
+  version: 994d9ab


Can you generate this file using v0.28.0 tag please? git pull --all --tags should help i beleive

I regen'd but I used the main branch version, because the v0.28.0 tag does not seem to allow me to use aws_sdk_go_version: v1.44.93 and that comes with a bunch of unwanted changes.

test/e2e/tests/helper.py

a-hilaly · 2023-12-29T21:23:29Z

test/e2e/tests/test_vpc_endpoint_service_configuration.py

+    nlb = NetworkLoadBalancer("vpc-ep-service-test")
+    nlb.bootstrap()


Ideally you want to set this in tests/service_bootstrap.py and service_cleanup.py, same for L92

Addressed in commit 1ac2150

a-hilaly · 2023-12-29T21:24:01Z

test/e2e/tests/test_vpc_endpoint_service_configuration.py

+        _, deleted = k8s.delete_custom_resource(ref, 3, 10)
+        assert deleted


We might wanna call the validator after this to assert that VPCESC is indeeded deleted from AWS

We do this a little lower at L136, in def test_vpc_endpoint_service_configuration_create_delete function.

pkg/resource/vpc_endpoint_service_configuration/hooks.go

Co-authored-by: Amine <hilalyamine@gmail.com>

a-hilaly

Thank you very much, sir!
/approve
/lgtm

ack-prow · 2024-01-03T22:49:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, marcdavoli

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [a-hilaly]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Merged PRs since last release: - #167 - #168 - #172 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ib-ak

… functionality (aws-controllers-k8s#172) Related to: - aws-controllers-k8s/community#1711 Description of changes: This is the continuation of PR aws-controllers-k8s#134 (Credit to @ib-ak !). This PR: - Adds the `VPCEndpointService` custom resource from the code-generator - Adds the field `spec.allowedPrincipals` of type `[]string`, which allows the user to specify a list of ARNs to share the VPC Endpoint Service with. Any Principals added to this list will have the VPC Endpoint Service shared with them. This field also ensures that Principals that are removed from the list have the VPC Endpoint Service share removed. - Tests are added for creation/deletion and for adding/deleting Principals to the Allowed List By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Merged PRs since last release: - aws-controllers-k8s#167 - aws-controllers-k8s#168 - aws-controllers-k8s#172 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

feat(VPCEndpointService): add custom resource

032df37

ack-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2023

ack-prow bot requested review from LikithaVemulapalli and a-hilaly December 21, 2023 18:42

ack-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 21, 2023

ack-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 21, 2023

marcdavoli added 7 commits December 21, 2023 13:48

chore: add e2e tests

ec2a4c9

chore: add hooks

69d507b

chore: add hooks.go

b1533e5

test: remove all tests except those for VPC Endpoint Service

c54a0a2

Merge remote-tracking branch 'upstream/main' into feat(VPCEndpointSer…

4c4fc09

…vice)/add-custom-resource

chore: add AllowPrincipals field of type []string

639ccde

fix: add AllowPrincipals field from ModifyVpcEndpointServicePermissio…

1ef97e6

…ns operation

marcdavoli changed the title ~~feat(VPCEndpointService): add custom resource~~ feat(VPCEndpointService): add custom resource and allowPrincipals and verifyPrivateDnsName functionality Dec 21, 2023

chore: add VerifyPrivateDNSName field of type bool

c3a32a8

marcdavoli changed the title ~~feat(VPCEndpointService): add custom resource and allowPrincipals and verifyPrivateDnsName functionality~~ feat(VPCEndpointService): add custom resource and allowPrincipals functionality Dec 21, 2023

marcdavoli and others added 11 commits December 21, 2023 16:00

revert: add VerifyPrivateDNSName field of type bool

028d520

chore: add logic for and rename field to AllowedPrincipals

8c737a4

chore: add logic for and rename field to AllowedPrincipals

1a86426

chore: add logic to add and remove principals

8c3a842

Merge branch 'main' into feat(VPCEndpointService)/add-custom-resource

f9c8ab8

feat(VPCEndpointService): add custom resource

6117a9c

chore: add e2e tests

1188ac8

chore: add hooks

7f780d1

chore: add hooks.go

ad08cf9

test: remove all tests except those for VPC Endpoint Service

84d0279

chore: add AllowPrincipals field of type []string

a9006f9

marcdavoli added 3 commits December 29, 2023 12:18

chore: put logic in syncAllowedPrincipals func

18985be

chore: put sdkRead Permissions logic in setAdditionalFields func

3c42c79

chore: put logic in syncAllowedPrincipals func

4f143d2

a-hilaly approved these changes Dec 29, 2023

View reviewed changes

ack-prow bot added the approved label Dec 29, 2023

marcdavoli added 5 commits January 2, 2024 15:11

chore: slightly refactor code to resemble iam-controller logic

ecf5d51

fix: regen code with code-gen v0.28.0

f89546a

test: remove duplicate functions

8dd97c8

test: move nlb booststrap logic to service_bootstrap.py

1ac2150

fix: missing ref

b21bcb7

a-hilaly reviewed Jan 3, 2024

View reviewed changes

marcdavoli and others added 10 commits January 2, 2024 23:19

Update pkg/resource/vpc_endpoint_service_configuration/hooks.go

591d730

Co-authored-by: Amine <hilalyamine@gmail.com>

Update pkg/resource/vpc_endpoint_service_configuration/hooks.go

b32a5fc

Co-authored-by: Amine <hilalyamine@gmail.com>

chore: remove unneeded 'if'

66d1f14

Update pkg/resource/vpc_endpoint_service_configuration/hooks.go

419adbd

Co-authored-by: Amine <hilalyamine@gmail.com>

fix: := error

0e222d1

fix: := error

3e69844

test: use new test-infra repo commit

ebb92a2

test: use new test-infra repo commit

68dd871

fix: NLB param name

fd9ae88

fix: remove duplicates

f63a0af

a-hilaly approved these changes Jan 3, 2024

View reviewed changes

ack-prow bot assigned a-hilaly Jan 3, 2024

ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 3, 2024

ack-prow bot merged commit d037f58 into aws-controllers-k8s:main Jan 3, 2024

marcdavoli mentioned this pull request Jan 4, 2024

chore(v1.2.0): create new release #173

Merged

ack-prow bot pushed a commit that referenced this pull request Jan 4, 2024

chore(v1.2.0): create new release (#173)

3ac4413

Merged PRs since last release: - #167 - #168 - #172 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

		nlb = NetworkLoadBalancer("vpc-ep-service-test")
		nlb.bootstrap()

		_, deleted = k8s.delete_custom_resource(ref, 3, 10)
		assert deleted

Conversation

marcdavoli commented Dec 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ack-prow bot commented Dec 21, 2023

Uh oh!

a-hilaly commented Dec 21, 2023

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

marcdavoli Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

a-hilaly Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

marcdavoli Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

a-hilaly Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

marcdavoli Jan 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

ack-prow bot commented Jan 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marcdavoli commented Dec 21, 2023 •

edited

Loading