Skip to content

Conversation

@michaelawyu
Copy link
Collaborator

Description of your changes

As discussed offline, this PR moves the envelope CRDs to the v1beta1 API group.

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

N/A, API only change

Special notes for your reviewer

N/A

Signed-off-by: michaelawyu <chenyu1@microsoft.com>
@michaelawyu michaelawyu marked this pull request as ready for review May 1, 2025 02:32
@michaelawyu michaelawyu changed the title chores: move the envelope CRDs to the v1beta1 API group chore: move the envelope CRDs to the v1beta1 API group May 1, 2025
@codecov
Copy link

codecov bot commented May 1, 2025

Codecov Report

Attention: Patch coverage is 0% with 44 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
apis/placement/v1beta1/zz_generated.deepcopy.go 0.00% 44 Missing ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: michaelawyu <chenyu1@microsoft.com>
@michaelawyu
Copy link
Collaborator Author

The APIs are updated per offline discussion.

@ryanzhang-oss ryanzhang-oss requested a review from Copilot May 1, 2025 17:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates envelope CRDs from the v1alpha1 API group to the v1beta1 API group. Key changes include updating apiVersion fields in YAML examples, modifying the CRD schema (replacing the old spec field with data), and regenerating deepcopy functions for the new API version.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
examples/envelopes/namespaced.yaml Updated apiVersion to v1beta1 and replaced spec.manifests with data mapping
examples/envelopes/clusterscoped.yaml Updated apiVersion to v1beta1 and replaced spec.manifests with data mapping
config/crd/bases/placement.kubernetes-fleet.io_resourceenvelopes.yaml Updated version, added a schema definition for the data field, and provided descriptive text
config/crd/bases/placement.kubernetes-fleet.io_clusterresourceenvelopes.yaml Updated version and schema to use the new data field
apis/placement/v1beta1/* Updated package version and regenerated deepcopy functions for the new API group
apis/placement/v1beta1/envelope_types.go Changed package name and replaced EnvelopeSpec with Data in CRD type definitions
apis/placement/v1alpha1/zz_generated.deepcopy.go Removed deepcopy functions for types shifted to v1beta1
Comments suppressed due to low confidence (2)

apis/placement/v1alpha1/zz_generated.deepcopy.go:161

  • [nitpick] If the v1alpha1 API group is fully deprecated, consider removing legacy deepcopy functions to reduce long-term maintenance overhead.
func (in *ClusterResourceEnvelope) DeepCopyInto(out *ClusterResourceEnvelope) {

examples/envelopes/namespaced.yaml:1

  • Confirm that client code and documentation have been updated to account for the change from using the 'spec' field to the 'data' field for embedded manifests.
apiVersion: placement.kubernetes-fleet.io/v1beta1

Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
Copy link

Copilot AI May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding an inline comment that explains the structure and purpose of the new 'data' field, clarifying that each key typically represents a filename that identifies a manifest.

Suggested change
type: string
type: string
# The 'data' field contains manifests wrapped in this envelope.
# Each key in the 'data' object typically represents a filename identifying a manifest.

Copilot uses AI. Check for mistakes.
Spec EnvelopeSpec `json:"spec"`
// +kubebuilder:validation:MinProperties=1
// +kubebuilder:validation:MaxProperties=50
Data map[string]runtime.RawExtension `json:"data"`
Copy link

Copilot AI May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Ensure that the transition from using a dedicated EnvelopeSpec to a direct 'data' mapping is thoroughly documented in code comments and external documentation for consistency.

Copilot uses AI. Check for mistakes.
@ryanzhang-oss ryanzhang-oss merged commit 4d2d00f into kubefleet-dev:main May 1, 2025
15 of 16 checks passed
audrastump pushed a commit to audrastump/kubefleet that referenced this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants