Skip to content

Commit

Permalink
fix(PingSource): remove annotation based conversion logic (#5234)
Browse files Browse the repository at this point in the history 
- v1beta1 will not guaranteed to be roundtrip-able (in the case that jsonData cannot be marshalled into json, e.g. jsonData="hello"), but the cloudevent on the wire stays the same.
- duckv1.SourceSpec, TimeZone, Schedule will be populated when called by v1beta1 clients
- object with v1beta2-specific features (dataBase64, contentType beyond `application/json`) will not populate jsonData field when converted to v1beta1.
  • Loading branch information
@eclipselu
eclipselu authored Apr 13, 2021
1 parent 8f35d42 commit 7c0b477
Show file tree
Hide file tree
Showing 2 changed files with 252 additions and 225 deletions.
131 changes: 15 additions & 116 deletions pkg/apis/sources/v1beta1/ping_conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,13 @@ import (
"context"
"encoding/json"
"fmt"
"reflect"

cloudevents "github.com/cloudevents/sdk-go/v2"

"knative.dev/eventing/pkg/apis/sources/v1beta2"
"knative.dev/pkg/apis"
)

const (
// V1B1SpecAnnotationKey is used to indicate that a v1beta2 object is converted from v1beta1
// also it can be used to downgrade such object to v1beta1
V1B1SpecAnnotationKey = "pingsources.sources.knative.dev/v1beta1-spec"

// V1B2SpecAnnotationKey is used to indicate that a v1beta1 object is converted from v1beta2
// also it can be used to convert the v1beta1 object back to v1beta2, considering that v1beta2 introduces more features.
V1B2SpecAnnotationKey = "pingsources.sources.knative.dev/v1beta2-spec"
)

type message struct {
Body string `json:"body"`
}
Expand All @@ -61,35 +50,21 @@ func (source *PingSource) ConvertTo(ctx context.Context, obj apis.Convertible) e
sink.Status = v1beta2.PingSourceStatus{
SourceStatus: source.Status.SourceStatus,
}

// deep copy annotations to avoid mutation on source.ObjectMeta.Annotations
annotations := make(map[string]string)
for key, value := range source.GetAnnotations() {
annotations[key] = value
sink.Spec = v1beta2.PingSourceSpec{
SourceSpec: source.Spec.SourceSpec,
Schedule: source.Spec.Schedule,
Timezone: source.Spec.Timezone,
}

if isCreatedViaV1Beta2API(source) {
// try to unmarshal v1beta2.PingSource.Spec from V1B2SpecAnnotationKey
// key existence and json marshal error already checked in isCreatedViaV1Beta2API
v1beta2Spec := annotations[V1B2SpecAnnotationKey]
_ = json.Unmarshal([]byte(v1beta2Spec), &sink.Spec)
} else {
var err error
if sink.Spec, err = toV1Beta2Spec(&source.Spec); err != nil {
return err
}
// marshal and store v1beta1.PingSource.Spec into V1B1SpecAnnotationKey
// this is to help if we need to convert back to v1beta1.PingSource
v1beta1Spec, err := json.Marshal(source.Spec)
if source.Spec.JsonData != "" {
msg, err := makeMessage(source.Spec.JsonData)
if err != nil {
return fmt.Errorf("error marshalling source.Spec: %v, err: %v", source.Spec, err)
return fmt.Errorf("error converting jsonData to a higher version: %v", err)
}
annotations[V1B1SpecAnnotationKey] = string(v1beta1Spec)
sink.Spec.ContentType = cloudevents.ApplicationJSON
sink.Spec.Data = string(msg)
}

// we don't need this annotation in a v1beta2.PingSource object
delete(annotations, V1B2SpecAnnotationKey)
sink.SetAnnotations(annotations)
return nil
default:
return apis.ConvertToViaProxy(ctx, source, &v1beta2.PingSource{}, sink)
Expand All @@ -106,94 +81,18 @@ func (sink *PingSource) ConvertFrom(ctx context.Context, obj apis.Convertible) e
SourceStatus: source.Status.SourceStatus,
}

// deep copy annotations to avoid mutation on source.ObjectMeta.Annotations
annotations := make(map[string]string)
for key, value := range source.GetAnnotations() {
annotations[key] = value
}

if isV1Beta1AnnotationConsistentWithV1Beta2Spec(source) {
// errors already handled in isV1Beta1AnnotationConsistentWithV1Beta2Spec
v1beta1Spec := annotations[V1B1SpecAnnotationKey]
_ = json.Unmarshal([]byte(v1beta1Spec), &sink.Spec)
sink.Spec = PingSourceSpec{
SourceSpec: source.Spec.SourceSpec,
Schedule: source.Spec.Schedule,
Timezone: source.Spec.Timezone,
}

// marshal and store v1beta2.PingSource.Spec into V1B2SpecAnnotationKey
// this is to help if we need to convert back to v1beta2.PingSource
v1beta2Configuration, err := json.Marshal(source.Spec)
if err != nil {
return fmt.Errorf("error marshalling source.Spec: %v, err: %v", source.Spec, err)
if source.Spec.ContentType == cloudevents.ApplicationJSON {
sink.Spec.JsonData = source.Spec.Data
}
annotations[V1B2SpecAnnotationKey] = string(v1beta2Configuration)
// we don't need this annotation in a v1beta1.PingSource object
delete(annotations, V1B1SpecAnnotationKey)
sink.SetAnnotations(annotations)

return nil
default:
return apis.ConvertFromViaProxy(ctx, source, &v1beta2.PingSource{}, sink)
}
}

func toV1Beta2Spec(srcSpec *PingSourceSpec) (v1beta2.PingSourceSpec, error) {
targetSpec := v1beta2.PingSourceSpec{
SourceSpec: srcSpec.SourceSpec,
Schedule: srcSpec.Schedule,
Timezone: srcSpec.Timezone,
}

if srcSpec.JsonData != "" {
msg, err := makeMessage(srcSpec.JsonData)
if err != nil {
return targetSpec, fmt.Errorf("error converting jsonData to a higher version: %v", err)
}
targetSpec.ContentType = cloudevents.ApplicationJSON
targetSpec.Data = string(msg)
}

return targetSpec, nil
}

// checks if a v1beta1.PingSource is originally created in v1beta2, it must meet both of the following criteria:
//
// 1. V1B2SpecAnnotationKey annotation must exist and can be unmarshalled to v1beta2.PingSourceSpec, it indicates that it's converted from v1beta2 -> v1beta1.
// 2. Spec.Sink must be {Ref: nil, URI: nil}, as we don't set these values during conversion from v1beta2 -> v1beta1, see PingSource.ConvertFrom;
func isCreatedViaV1Beta2API(source *PingSource) bool {
v1beta2Annotation, ok := source.GetAnnotations()[V1B2SpecAnnotationKey]
if !ok {
return false
}

v1beta2Spec := &v1beta2.PingSourceSpec{}
if err := json.Unmarshal([]byte(v1beta2Annotation), v1beta2Spec); err != nil {
return false
}

return source.Spec.Sink.Ref == nil && source.Spec.Sink.URI == nil
}

// for a v1beta2.PingSource, checks if its V1B1SpecAnnotationKey is consistent with its spec.
// returns false if one of the following satisfies:
//
// 1. V1B1SpecAnnotationKey does not exist.
// 2. V1B1SpecAnnotationKey exists, but we cannot unmarshal it to v1beta1.PingSourceSpec.
// 3. V1B1SpecAnnotationKey exists, but if we unmarshal it to v1beta1.PingSourceSpec and convert it to v1beta2,
// the converted v1beta2.PingSourceSpec is not the same as source.Spec.
func isV1Beta1AnnotationConsistentWithV1Beta2Spec(source *v1beta2.PingSource) bool {
v1beta1Annotation, ok := source.GetAnnotations()[V1B1SpecAnnotationKey]
if !ok {
return false
}

v1beta1Spec := &PingSourceSpec{}
if err := json.Unmarshal([]byte(v1beta1Annotation), v1beta1Spec); err != nil {
return false
}

v1beta2Spec, err := toV1Beta2Spec(v1beta1Spec)
if err != nil {
return false
}

return reflect.DeepEqual(v1beta2Spec, source.Spec)
}
Loading

0 comments on commit 7c0b477

Please sign in to comment.