Add support for platform 010 by natalieparellano · Pull Request #1332 · buildpacks-community/kpack

natalieparellano · 2023-09-27T21:47:44Z

There are still some open TODOs but this is ready for detailed feedback

Summary of larger changes

Configures the build pod for extensions when they are present in the order (note changes to the security context such as running as root and added capabilities)
Adds new resources and reconcilers for Extensions and ClusterExtensions for parity with Buildpacks and ClusterBuildpacks
Refactors “buildpack resolver” to reduce duplication

Unresolved

The integration tests will fail until the lifecycle version is bumped to 0.18.0+ due to a regression in 0.17.x that broke Platform 0.10 - we should bump the lifecycle version in this PR when 0.18.0 is out (likely this or next week)
Platforms can opt-in to using extensions by specifying them in the builder. Do we need any additional opting-in like setting “experimental = true” somewhere?
Note that rebuilds on stack changes are disabled when extensions are used (see here) - this should be improved later but I didn’t want to do it in this PR to avoid making the changeset even larger
See other small questions in the conversations below

Signed-off-by: Natalie Arellano <narellano@vmware.com>

codecov-commenter · 2023-09-27T21:55:04Z

Codecov Report

Merging #1332 (d6036e4) into main (6f4bd45) will decrease coverage by 0.17%.
Report is 2 commits behind head on main.
The diff coverage is 66.79%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main    #1332      +/-   ##
==========================================
- Coverage   67.34%   67.17%   -0.17%     
==========================================
  Files         133      138       +5     
  Lines        8210     8503     +293     
==========================================
+ Hits         5529     5712     +183     
- Misses       2231     2343     +112     
+ Partials      450      448       -2

Files	Coverage Δ
pkg/apis/build/v1alpha2/build.go	`28.16% <ø> (+1.14%)`	⬆️
pkg/apis/build/v1alpha2/build_conversion.go	`94.59% <100.00%> (ø)`
pkg/apis/build/v1alpha2/build_pod.go	`97.18% <100.00%> (+0.10%)`	⬆️
pkg/apis/build/v1alpha2/build_types.go	`100.00% <ø> (ø)`
pkg/apis/build/v1alpha2/builder_conversion.go	`94.59% <100.00%> (ø)`
pkg/apis/build/v1alpha2/builder_types.go	`22.22% <ø> (ø)`
pkg/apis/build/v1alpha2/builder_validation.go	`88.42% <100.00%> (+3.91%)`	⬆️
pkg/apis/build/v1alpha2/buildpack_validation.go	`100.00% <ø> (ø)`
pkg/apis/build/v1alpha2/image_builds.go	`69.48% <ø> (ø)`
pkg/buildchange/buildpack_change.go	`82.35% <100.00%> (ø)`
... and 26 more

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano · 2023-09-28T17:53:04Z

Still to do:

Add reconcilers for Extensions and ClusterExtensions
Add end-to-end test(s)
Actually use extensions in the build pod

Before I proceed further, I'd like to confirm that we indeed want to add these additional resources ^^ (Slack thread)

Maintainers, please opine!

natalieparellano · 2023-10-04T21:20:50Z

Makefile


 e2e:
-	$(GOCMD) test --timeout=30m -v ./test/...
+	$(GOCMD) test --timeout=30m -failfast -v ./test/...


We don't have to add this... but I like it

go.sum

Signed-off-by: Natalie Arellano <narellano@vmware.com>

go.sum

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano · 2023-10-16T17:45:16Z

pkg/git/url_parser_test.go


 func TestParseURL(t *testing.T) {
-	spec.Focus(t, "Test Parse Git URL", testParseURL)
+	spec.Focus(t, "Test Parse Git URL", testParseURL) // TODO: should this be .Focus?


Is this a forgotten .Focus?

I suspect that it works because its the only thing in the function but it probably should not be a focus

natalieparellano · 2023-10-16T17:49:12Z

test/execute_build_test.go

+			},
+			Spec: buildapi.ExtensionSpec{
+				ImageSource: corev1alpha1.ImageSource{
+					Image: "natalieparellano/sample-extension", // FIXME


Perhaps we could build the sample extension and push it along with all the other images from hack/local.sh or equivalent?

that feels slightly odd to me since we don't do that with buildpacks. maybe we can put a sample extension in the projects ghcr repo or something

I would prefer to use a properly released (read: maintained) extension, does CNB have a sample one? What about paketo or other well known buildpack authors?

natalieparellano · 2023-10-16T20:50:50Z

pkg/reconciler/builder/builder_test.go

+			//require.True(t, fakeTracker.IsTrackingKind(
+			//	kreconciler.KeyForObject(extension).GroupKind,
+			//	builder.NamespacedName()))
+			//require.True(t, fakeTracker.IsTrackingKind(
+			//	kreconciler.KeyForObject(clusterExtension).GroupKind,
+			//	builder.NamespacedName()))


I have no idea what this is testing or why it's failing. Maintainers, please help!

The tracker is a thing we use to force reconciliation of related objects. The usage is that it is first added as an event handler to a specific informer, and then during each reconcile loop we can decide to register individual things to track. Then when these things get modified (by the user or other controllers), it triggers a reconcile loop on a separate reconciler.

To give an example, the Builder reconciler sets up a tracker for all Buildpack objects. This is so that if a buildpack was changed (i.e. user updated the .spec.image field), we want to force a reconciliation of any builders that uses it:

kpack/pkg/reconciler/builder/builder.go

Lines 97 to 101 in b14fce9

buildpackInformer.Informer().AddEventHandler(controller.HandleAll(

controller.EnsureTypeMeta(

c.Tracker.OnChanged,

buildapi.SchemeGroupVersion.WithKind(buildapi.BuildpackKind)),

))

Once the tracker is set to handle events for all Buildpacks, it can register individual objects (in this case a ClusterStore) for tracking

kpack/pkg/reconciler/builder/builder.go

Lines 175 to 184 in b14fce9

c.Tracker.Track(reconciler.Key{

NamespacedName: types.NamespacedName{

Name: builder.Spec.Store.Name,

Namespace: metav1.NamespaceAll,

},

GroupKind: schema.GroupKind{

Group: "kpack.io",

Kind: buildapi.ClusterStoreKind,

},

}, builder.NamespacedName())

Or track all group kind (in this case Buildpacks) in a namespace

kpack/pkg/reconciler/builder/builder.go

Lines 192 to 195 in b14fce9

c.Tracker.TrackKind(schema.GroupKind{

Group: "kpack.io",

Kind: buildapi.BuildpackKind,

}, builder.NamespacedName())

The test injects a fake tracker into the reconciler and asserts that it's setting up the expected namespaced-objects/group-version-kind to track.

So you'll probably have to setup the tracker in the builder reconciler to also watch for changes to the [Cluster]Extension resource. If you don't, the builder won't be reconciled if a new extension is added, or if an existing extension changes.

natalieparellano · 2023-10-16T20:51:23Z

pkg/reconciler/clusterbuilder/clusterbuilder_test.go

+			//require.True(t, fakeTracker.IsTrackingKind(
+			//	kreconciler.KeyForObject(clusterExtension).GroupKind, builder.NamespacedName()))


I have no idea what this is testing or why it's failing. Maintainers, please help!

Same as #1332 (comment), with the only difference being that it only watches for ClusterExtensions instead of both Extension and ClusterExtension

Signed-off-by: Natalie Arellano <narellano@vmware.com>

pkg/apis/build/v1alpha2/build.go

natalieparellano · 2023-10-16T21:00:00Z

pkg/reconciler/image/build_required.go

 		return nil
 	}

+	if len(builder.ExtensionMetadata()) > 0 {


Without this, if extensions are used to switch the runtime base image, the image will never stop building because the reconciler always sees a stack change. Future versions of kpack should improve this by (somehow) tying the image back to a "stack" that includes the runtime base image that was switched to.

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano · 2023-10-17T13:24:34Z

The integration tests won't run in CI until this is merged, but here is my local output if that's helpful:

PASS | KpackE2E (651.78s)
PASS |   KpackE2E/CreateImage (547.52s)
PASS |     KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources (36.75s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/blob-image-custom-builder (74.87s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/registry-image-custom-cluster-builder-with-extensions (84.18s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/registry-image-custom-builder (85.01s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/git-image-custom-cluster-builder-with-extensions (87.26s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/registry-image-custom-cluster-builder (90.64s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/git-image-custom-builder-with-extensions (124.00s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/blob-image-custom-cluster-builder (134.21s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/blob-image-custom-builder-with-extensions (156.83s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/git-image-custom-builder (157.49s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/blob-image-custom-cluster-builder-with-extensions (170.88s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/registry-image-custom-builder-with-extensions (174.89s)
PASS |       KpackE2E/CreateImage/builds_and_rebases_git,_blob,_and_registry_images_from_unauthenticated_sources/git-image-custom-cluster-builder (193.01s)
PASS |     KpackE2E/CreateImage/builds_and_rebases_git_sources_from_authenticated_sources (22.24s)
PASS |     KpackE2E/CreateImage/can_trigger_rebuilds_with_volume_cache (136.48s)
PASS |     KpackE2E/CreateImage/can_trigger_rebuilds_with_registry_cache (159.04s)
PASS |   KpackE2E/SignBuilder (104.26s)
PASS |     KpackE2E/SignBuilder/Signs_a_Builder_image_successfully_when_the_key_is_not_password-protected (15.04s)
PASS |     KpackE2E/SignBuilder/Signs_a_Builder_image_successfully_when_the_key_is_password-protected (13.44s)
PASS |     KpackE2E/SignBuilder/Generates_more_than_one_signature_for_a_Builder_image_successfully_when_multiple_secrets_are_present (16.17s)
PASS |     KpackE2E/SignBuilder/Saves_a_failure_in_the_Builder_record_when_signing_fails (9.76s)
PASS |     KpackE2E/SignBuilder/Signs_a_ClusterBuilder_image_successfully_when_the_key_is_not_password-protected (8.60s)
PASS |     KpackE2E/SignBuilder/Signs_a_ClusterBuilder_image_successfully_when_the_key_is_password-protected (13.19s)
PASS |     KpackE2E/SignBuilder/Generates_more_than_one_signature_for_a_ClusterBuilder_image_successfully_when_multiple_secrets_are_present (17.22s)
PASS |     KpackE2E/SignBuilder/Saves_a_failure_in_the_ClusterBuilder_record_when_signing_fails (10.85s)
PASS | github.com/pivotal/kpack/test 652.960s

natalieparellano · 2023-10-31T14:20:10Z

pkg/apis/build/v1alpha2/build_pod.go

+			container.SecurityContext.RunAsUser = intPointer(0)
+			container.SecurityContext.RunAsGroup = intPointer(0)
+			container.SecurityContext.RunAsNonRoot = boolPointer(false)
+			container.SecurityContext.Capabilities = &corev1.Capabilities{Add: []corev1.Capability{"SETGID", "SETUID"}}


Discussed in 10/31 Working Group, run as root is unnecessary if all we're doing is switching the runtime base. We should make this configurable by the end user (and default to the most secure setting).

natalieparellano · 2023-11-13T16:31:20Z

@tomkennedy513 @chenbh did you have a chance to go through all the changes? If so I can pick this up again

Natalie Arellano added 10 commits September 26, 2023 14:02

First create builder unit passes

2a2ab54

Signed-off-by: Natalie Arellano <narellano@vmware.com>

More assertions

5faeae0

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add directives for Extensions and ClusterExtensions

44b354f

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Generate some code

b95b212

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add extension validations

512c1d2

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Feed extensions to remote buildpack fetcher

11552f1

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add empty "resolve extension" method

fd7ab30

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Units pass

34fcf4a

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add builder validations

ebecedc

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add resolve extension

b5ae35c

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Natalie Arellano added 5 commits September 28, 2023 09:20

Add test for ref image

8ed1ecc

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Remove duplication by adding ModuleResource interface

71e25d2

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Remove TODO

2663049

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Return a useful error on Windows

6a60008

Signed-off-by: Natalie Arellano <narellano@vmware.com>

Add extension reader

dd5c115

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the add-platform-010 branch from e735f8a to 16515e9 Compare October 4, 2023 21:19

natalieparellano commented Oct 4, 2023

View reviewed changes

go.sum Outdated Show resolved Hide resolved

Add basic e2e test for extensions

160ce19

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the add-platform-010 branch from 16515e9 to 160ce19 Compare October 4, 2023 21:33

WIP: add integration test for build with extensions

dbaa66d

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano force-pushed the add-platform-010 branch from 98befe4 to dbaa66d Compare October 13, 2023 19:42

Merge branch 'main' into add-platform-010

8994382

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano commented Oct 16, 2023

View reviewed changes

go.sum Outdated Show resolved Hide resolved

Update go.sum

a029735

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano commented Oct 16, 2023

View reviewed changes

Tests seem to pass

d6036e4

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano commented Oct 16, 2023

View reviewed changes

pkg/apis/build/v1alpha2/build.go Show resolved Hide resolved

natalieparellano commented Oct 16, 2023

View reviewed changes

Fix reconciler

b14fce9

Signed-off-by: Natalie Arellano <narellano@vmware.com>

natalieparellano marked this pull request as ready for review October 17, 2023 13:22

natalieparellano requested a review from a team as a code owner October 17, 2023 13:22

natalieparellano commented Oct 31, 2023

View reviewed changes

natalieparellano mentioned this pull request Nov 3, 2023

Looking to call the extender from within the creator buildpacks/lifecycle#1155

Open

chenbh mentioned this pull request Oct 17, 2024

Update to support 0.10 of the Buildpack API #1730

Closed

	buildpackInformer.Informer().AddEventHandler(controller.HandleAll(
	controller.EnsureTypeMeta(
	c.Tracker.OnChanged,
	buildapi.SchemeGroupVersion.WithKind(buildapi.BuildpackKind)),
	))

	c.Tracker.Track(reconciler.Key{
	NamespacedName: types.NamespacedName{
	Name: builder.Spec.Store.Name,
	Namespace: metav1.NamespaceAll,
	},
	GroupKind: schema.GroupKind{
	Group: "kpack.io",
	Kind: buildapi.ClusterStoreKind,
	},
	}, builder.NamespacedName())

	c.Tracker.TrackKind(schema.GroupKind{
	Group: "kpack.io",
	Kind: buildapi.BuildpackKind,
	}, builder.NamespacedName())

		//require.True(t, fakeTracker.IsTrackingKind(
		// kreconciler.KeyForObject(clusterExtension).GroupKind, builder.NamespacedName()))

Conversation

natalieparellano commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of larger changes

Unresolved

Uh oh!

codecov-commenter commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

natalieparellano commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natalieparellano commented Oct 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natalieparellano commented Nov 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

natalieparellano commented Sep 27, 2023 •

edited

Loading

codecov-commenter commented Sep 27, 2023 •

edited

Loading

natalieparellano commented Sep 28, 2023 •

edited

Loading