Skip to content

Commit 3c7647a

Browse files
committed
create new APIResourceSchemas when CRDs or PRs change, improve mutation logic accessibility
On-behalf-of: @SAP [email protected]
1 parent ce77504 commit 3c7647a

File tree

4 files changed

+191
-80
lines changed

4 files changed

+191
-80
lines changed

internal/controller/apiresourceschema/controller.go

Lines changed: 13 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"fmt"
2222
"reflect"
23-
"strings"
2423

2524
"github.com/kcp-dev/logicalcluster/v3"
2625
"go.uber.org/zap"
@@ -122,31 +121,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
122121

123122
func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubResource *syncagentv1alpha1.PublishedResource) (*reconcile.Result, error) {
124123
// find the resource that the PublishedResource is referring to
125-
localGVK := projection.PublishedResourceSourceGVK(pubResource)
124+
localGK := projection.PublishedResourceSourceGK(pubResource)
126125

127126
client, err := discovery.NewClient(r.restConfig)
128127
if err != nil {
129128
return nil, fmt.Errorf("failed to create discovery client: %w", err)
130129
}
131130

132-
crd, err := client.RetrieveCRD(ctx, localGVK)
131+
// fetch the original, full CRD from the cluster
132+
crd, err := client.RetrieveCRD(ctx, localGK)
133133
if err != nil {
134134
return nil, fmt.Errorf("failed to discover resource defined in PublishedResource: %w", err)
135135
}
136136

137-
// project the CRD
138-
projectedCRD, err := r.applyProjection(crd, pubResource)
137+
// project the CRD (i.e. strip unwanted versions, rename values etc.)
138+
projectedCRD, err := projection.ApplyProjection(crd, pubResource)
139139
if err != nil {
140140
return nil, fmt.Errorf("failed to apply projection rules: %w", err)
141141
}
142142

143-
// to prevent changing the source GVK e.g. from "apps/v1 Daemonset" to "core/v1 Pod",
144-
// we include the source GVK in hashed form in the final APIResourceSchema name.
143+
// generate a unique name for this exact state of the CRD
145144
arsName := r.getAPIResourceSchemaName(projectedCRD)
146145

147-
// ARS'es cannot be updated, their entire spec is immutable. For now we do not care about
148-
// CRDs being updated on the service cluster, but in the future (TODO) we must allow
149-
// service owners to somehow publish updated CRDs without changing their API version.
146+
// ensure ARS exists (don't try to reconcile it, it's basically entirely immutable)
150147
wsCtx := kontext.WithCluster(ctx, r.lcName)
151148
ars := &kcpdevv1alpha1.APIResourceSchema{}
152149
err = r.kcpClient.Get(wsCtx, types.NamespacedName{Name: arsName}, ars, &ctrlruntimeclient.GetOptions{})
@@ -159,7 +156,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR
159156
return nil, fmt.Errorf("failed to check for APIResourceSchema: %w", err)
160157
}
161158

162-
// Update Status with ARS name
159+
// update Status with ARS name
163160
if pubResource.Status.ResourceSchemaName != arsName {
164161
original := pubResource.DeepCopy()
165162
pubResource.Status.ResourceSchemaName = arsName
@@ -176,7 +173,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR
176173
}
177174

178175
func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.SugaredLogger, projectedCRD *apiextensionsv1.CustomResourceDefinition, arsName string) error {
179-
// prefix is irrelevant as the reconciling framework will use arsName anyway
176+
// prefix is irrelevant as the name is overridden later
180177
converted, err := kcpdevv1alpha1.CRDToAPIResourceSchema(projectedCRD, "irrelevant")
181178
if err != nil {
182179
return fmt.Errorf("failed to convert CRD: %w", err)
@@ -198,59 +195,13 @@ func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.Sugar
198195
return r.kcpClient.Create(ctx, ars)
199196
}
200197

201-
func (r *Reconciler) applyProjection(crd *apiextensionsv1.CustomResourceDefinition, pr *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) {
202-
result := crd.DeepCopy()
203-
204-
// Currently CRDs generated by our discovery mechanism already set these to true, but that's just
205-
// because it doesn't care to set them correctly; we keep this code here because from here on,
206-
// in kcp, we definitely want them to be true.
207-
result.Spec.Versions[0].Served = true
208-
result.Spec.Versions[0].Storage = true
209-
210-
projection := pr.Spec.Projection
211-
if projection == nil {
212-
return result, nil
213-
}
214-
215-
if projection.Group != "" {
216-
result.Spec.Group = projection.Group
217-
}
218-
219-
if projection.Version != "" {
220-
result.Spec.Versions[0].Name = projection.Version
221-
}
222-
223-
if projection.Kind != "" {
224-
result.Spec.Names.Kind = projection.Kind
225-
result.Spec.Names.ListKind = projection.Kind + "List"
226-
227-
result.Spec.Names.Singular = strings.ToLower(result.Spec.Names.Kind)
228-
result.Spec.Names.Plural = result.Spec.Names.Singular + "s"
229-
}
230-
231-
if projection.Plural != "" {
232-
result.Spec.Names.Plural = projection.Plural
233-
}
234-
235-
if projection.Scope != "" {
236-
result.Spec.Scope = apiextensionsv1.ResourceScope(projection.Scope)
237-
}
238-
239-
if projection.Categories != nil {
240-
result.Spec.Names.Categories = projection.Categories
241-
}
242-
243-
if projection.ShortNames != nil {
244-
result.Spec.Names.ShortNames = projection.ShortNames
245-
}
246-
247-
return result, nil
248-
}
249-
250198
// getAPIResourceSchemaName generates the name for the ARS in kcp. Note that
251199
// kcp requires, just like CRDs, that ARS are named following a specific pattern.
252200
func (r *Reconciler) getAPIResourceSchemaName(crd *apiextensionsv1.CustomResourceDefinition) string {
253-
checksum := crypto.Hash(crd.Spec.Names)
201+
crd = crd.DeepCopy()
202+
crd.Spec.Conversion = nil
203+
204+
checksum := crypto.Hash(crd.Spec)
254205

255206
// include a leading "v" to prevent SHA-1 hashes with digits to break the name
256207
return fmt.Sprintf("v%s.%s.%s", checksum[:8], crd.Spec.Names.Plural, crd.Spec.Group)

internal/discovery/client.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte
156156
crd.ObjectMeta = metav1.ObjectMeta{
157157
Name: oldMeta.Name,
158158
Annotations: filterAnnotations(oldMeta.Annotations),
159+
Generation: oldMeta.Generation, // is stored as an annotation for convenience on the ARS
159160
}
160161
crd.Status.Conditions = []apiextensionsv1.CustomResourceDefinitionCondition{}
161162

@@ -232,9 +233,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte
232233
// fill-in the schema for each version, making sure that versions are sorted
233234
// according to Kubernetes rules.
234235
sortedVersions := availableVersions.UnsortedList()
235-
slices.SortFunc(sortedVersions, func(a, b string) int {
236-
return version.CompareKubeAwareVersionStrings(a, b)
237-
})
236+
slices.SortFunc(sortedVersions, version.CompareKubeAwareVersionStrings)
238237

239238
for _, version := range sortedVersions {
240239
subresources := subresourcesPerVersion[version]

internal/projection/projection.go

Lines changed: 171 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,25 @@ limitations under the License.
1717
package projection
1818

1919
import (
20+
"errors"
21+
"fmt"
22+
"slices"
23+
"strings"
24+
2025
syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1"
2126

27+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2228
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/apimachinery/pkg/util/sets"
30+
"k8s.io/apimachinery/pkg/version"
2331
)
2432

25-
// PublishedResourceSourceGVK returns the source GVK of the local resources
33+
// PublishedResourceSourceGK returns the source GK of the local resources
2634
// that are supposed to be published.
27-
func PublishedResourceSourceGVK(pubRes *syncagentv1alpha1.PublishedResource) schema.GroupVersionKind {
28-
return schema.GroupVersionKind{
29-
Group: pubRes.Spec.Resource.APIGroup,
30-
Version: pubRes.Spec.Resource.Version,
31-
Kind: pubRes.Spec.Resource.Kind,
35+
func PublishedResourceSourceGK(pubRes *syncagentv1alpha1.PublishedResource) schema.GroupKind {
36+
return schema.GroupKind{
37+
Group: pubRes.Spec.Resource.APIGroup,
38+
Kind: pubRes.Spec.Resource.Kind,
3239
}
3340
}
3441

@@ -59,3 +66,161 @@ func PublishedResourceProjectedGVK(pubRes *syncagentv1alpha1.PublishedResource)
5966
Kind: kind,
6067
}
6168
}
69+
70+
func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) {
71+
result := crd.DeepCopy()
72+
73+
// reduce the CRD down to the selected versions
74+
result, err := stripUnwantedVersions(result, pubRes)
75+
if err != nil {
76+
return nil, err
77+
}
78+
79+
// if there is no storage version left, we use the latest served version
80+
result, err = adjustStorageVersion(result)
81+
if err != nil {
82+
return nil, err
83+
}
84+
85+
// now we get to actually project something, if desired
86+
result, err = projectCRD(result, pubRes)
87+
if err != nil {
88+
return nil, err
89+
}
90+
91+
return result, nil
92+
}
93+
94+
func stripUnwantedVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) {
95+
src := pubRes.Spec.Resource
96+
97+
if src.Version != "" && len(src.Versions) > 0 {
98+
return nil, errors.New("cannot configure both .version and .versions in as the source of a PublishedResource")
99+
}
100+
101+
crd.Spec.Versions = slices.DeleteFunc(crd.Spec.Versions, func(ver apiextensionsv1.CustomResourceDefinitionVersion) bool {
102+
switch {
103+
case src.Version != "":
104+
return ver.Name != src.Version
105+
case len(src.Versions) > 0:
106+
return !slices.Contains(src.Versions, ver.Name)
107+
default:
108+
return false // i.e. keep all versions by default
109+
}
110+
})
111+
112+
if len(crd.Spec.Versions) == 0 {
113+
switch {
114+
case src.Version != "":
115+
return nil, fmt.Errorf("CRD does not contain version %s", src.Version)
116+
case len(src.Versions) > 0:
117+
return nil, fmt.Errorf("CRD does not contain any of versions %v", src.Versions)
118+
default:
119+
return nil, errors.New("CRD contains no versions")
120+
}
121+
}
122+
123+
return crd, nil
124+
}
125+
126+
func adjustStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (*apiextensionsv1.CustomResourceDefinition, error) {
127+
var hasStorage bool
128+
latestServed := -1
129+
for i, v := range crd.Spec.Versions {
130+
if v.Storage {
131+
hasStorage = true
132+
}
133+
if v.Served {
134+
latestServed = i
135+
}
136+
}
137+
138+
if latestServed < 0 {
139+
return nil, errors.New("no CRD version selected that is marked as served")
140+
}
141+
142+
if !hasStorage {
143+
crd.Spec.Versions[latestServed].Storage = true
144+
}
145+
146+
return crd, nil
147+
}
148+
149+
func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) {
150+
projection := pubRes.Spec.Projection
151+
if projection == nil {
152+
return crd, nil
153+
}
154+
155+
if projection.Group != "" {
156+
crd.Spec.Group = projection.Group
157+
}
158+
159+
indexOfVersion := func(v string) int {
160+
for i, version := range crd.Spec.Versions {
161+
if version.Name == v {
162+
return i
163+
}
164+
}
165+
166+
return -1
167+
}
168+
169+
// We already validated that Version and Versions can be set at the same time.
170+
171+
if projection.Version != "" {
172+
if size := len(crd.Spec.Versions); size != 1 {
173+
return nil, fmt.Errorf("cannot project CRD version to a single version %q because it contains %d versions", projection.Version, size)
174+
}
175+
176+
crd.Spec.Versions[0].Name = projection.Version
177+
} else if len(projection.Versions) > 0 {
178+
for _, mut := range projection.Versions {
179+
fromIdx := indexOfVersion(mut.From)
180+
if fromIdx < 0 {
181+
return nil, fmt.Errorf("cannot project CRD version %s to %s because there is no %s version", mut.From, mut.To, mut.From)
182+
}
183+
184+
crd.Spec.Versions[fromIdx].Name = mut.To
185+
}
186+
187+
// ensure we ended up with a unique set of versions
188+
knownVersions := sets.New[string]()
189+
for _, version := range crd.Spec.Versions {
190+
if knownVersions.Has(version.Name) {
191+
return nil, fmt.Errorf("CRD contains multiple entries for %s after applying mutation rules", version.Name)
192+
}
193+
}
194+
195+
// ensure proper Kubernetes-style version order
196+
slices.SortFunc(crd.Spec.Versions, func(a, b apiextensionsv1.CustomResourceDefinitionVersion) int {
197+
return version.CompareKubeAwareVersionStrings(a.Name, b.Name)
198+
})
199+
}
200+
201+
if projection.Kind != "" {
202+
crd.Spec.Names.Kind = projection.Kind
203+
crd.Spec.Names.ListKind = projection.Kind + "List"
204+
205+
crd.Spec.Names.Singular = strings.ToLower(crd.Spec.Names.Kind)
206+
crd.Spec.Names.Plural = crd.Spec.Names.Singular + "s"
207+
}
208+
209+
if projection.Plural != "" {
210+
crd.Spec.Names.Plural = projection.Plural
211+
}
212+
213+
if projection.Scope != "" {
214+
crd.Spec.Scope = apiextensionsv1.ResourceScope(projection.Scope)
215+
}
216+
217+
if projection.Categories != nil {
218+
crd.Spec.Names.Categories = projection.Categories
219+
}
220+
221+
if projection.ShortNames != nil {
222+
crd.Spec.Names.ShortNames = projection.ShortNames
223+
}
224+
225+
return crd, nil
226+
}

internal/projection/projection_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,14 @@ func TestPublishedResourceSourceGVK(t *testing.T) {
4141
},
4242
}
4343

44-
gvk := PublishedResourceSourceGVK(&pubRes)
44+
gk := PublishedResourceSourceGK(&pubRes)
4545

46-
if gvk.Group != apiGroup {
47-
t.Errorf("Expected API group to be %q, but got %q.", apiGroup, gvk.Group)
46+
if gk.Group != apiGroup {
47+
t.Errorf("Expected API group to be %q, but got %q.", apiGroup, gk.Group)
4848
}
4949

50-
if gvk.Version != version {
51-
t.Errorf("Expected version to be %q, but got %q.", version, gvk.Version)
52-
}
53-
54-
if gvk.Kind != kind {
55-
t.Errorf("Expected kind to be %q, but got %q.", kind, gvk.Kind)
50+
if gk.Kind != kind {
51+
t.Errorf("Expected kind to be %q, but got %q.", kind, gk.Kind)
5652
}
5753
}
5854

0 commit comments

Comments
 (0)