Kubernetes controller lib: Unable to extract and merge by field manager

76 Views Asked by At

I understand what's wrong. I have looked around everywhere to figure out what the fix could be. Really need help for it. I have recreated the issue that you can easily run yourself: https://github.com/subtleseeker/extract-merge-issue/blob/main/pkg/myissue_test.go

I have a component that tries to recreate the resource yaml by only keeping the fields that are owned by the field managers that I require. To do that, I "extract" those fields for every field manager and "merge" it in a new object. One element of metadata.managedFields looks like:

{
                "apiVersion": "v1",
                "fieldsType": "FieldsV1",
                "fieldsV1": {
                    "f:spec": {
                        "f:ports": {
                            "k:{\"port\":80,\"protocol\":\"TCP\"}": {
                                "f:nodePort": {}
                            }
                        }
                    }
                },
                "manager": "kubectl-edit",
                "operation": "Update",
                "time": "2023-12-21T05:59:59Z"
            }

Here, after applying the yaml, I edit the field ports[0].nodePort with kubectl edit. The extract'ed value from the service yaml looks like:

{
    "spec": {
        "ports": [
            {
                "nodePort": 30001
            }
        ]
    }
}

If you see, this doesn't contain the key {\"port\":80,\"protocol\":\"TCP\"} that is required field during a merge operation. During merge, I see the error:

.spec.ports: element 0: associative list with keys has an element that omits key field "port" (and doesn't have default value)

This is some of the relevant code:

func TestIssue(t *testing.T) {
    ctx := context.Background()

    // Create new creator instance.
    r, err := New(ctx, cfg)
    if err != nil {
        panic(err)
    }

    // Service GVK.
    gvk := schema.GroupVersionKind{
        Group:   "",
        Version: "v1",
        Kind:    "Service",
    }

    // Create parseable type for service GVK.
    objectType := r.ParseableType(ctx, gvk)
    if objectType == nil {
        panic("failed to fetch the objectType: " + gvk.String())
    }
    if !objectType.IsValid() {
        panic("parseable type for GVK %v not valid" + gvk.String())
    }
    // logrus.Infof("objectType: %s", JsonObjectToString(objectType))

    // Service yaml for simulation:
    // The yaml was 'kubectl apply'ed followed by editting 'ports.nodeport'
    // field with 'kubectl edit'.
    // The thing to note is that there are thus 2 field managers:
    // - 'kubectl-client-side-apply': Owns everything.
    // - 'kubectl-edit': Shares ownership of the field 'ports.nodeport'.
    object := jsonToUnstructured(`{"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"managedFields":[{"apiVersion":"v1","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{".":{},"f:kubectl.kubernetes.io/last-applied-configuration":{}}},"f:spec":{"f:externalTrafficPolicy":{},"f:internalTrafficPolicy":{},"f:ports":{".":{},"k:{\"port\":80,\"protocol\":\"TCP\"}":{".":{},"f:name":{},"f:port":{},"f:protocol":{},"f:targetPort":{}}},"f:selector":{},"f:sessionAffinity":{},"f:type":{}}},"manager":"kubectl-client-side-apply","operation":"Update","time":"2023-12-21T05:29:51Z"},{"apiVersion":"v1","fieldsType":"FieldsV1","fieldsV1":{"f:spec":{"f:ports":{"k:{\"port\":80,\"protocol\":\"TCP\"}":{"f:nodePort":{}}}}},"manager":"kubectl-edit","operation":"Update","time":"2023-12-21T05:59:59Z"}],"name":"clear-nginx-service"},"spec":{"clusterIP":"172.19.41.134","clusterIPs":["172.19.41.134"],"externalTrafficPolicy":"Cluster","internalTrafficPolicy":"Cluster","ipFamilies":["IPv4"],"ipFamilyPolicy":"SingleStack","ports":[{"name":"http","nodePort":30001,"port":80,"protocol":"TCP","targetPort":80}],"selector":{"app":"clear-nginx"},"sessionAffinity":"None","type":"NodePort"}}`)

    objManagedFields := object.GetManagedFields()
    origObj, err := objectType.FromUnstructured(object.Object)

    for _, managedField := range objManagedFields {
        if managedField.Manager == "kubectl-client-side-apply" {
            // Simulation: The above object has gone through this already. Thus, continue.
            continue
        }
        // Reaching here means: The managedField here is kubectl-edit.
        logrus.Infof("managedField: %v", managedField.Manager)
        field := &managedField
        fieldset := &fieldpath.Set{}
        err := fieldset.FromJSON(bytes.NewReader(field.FieldsV1.Raw))
        if err != nil {
            panic(err)
        }

        logrus.Info("original object before extracting fields", "origObject", origObj.AsValue())
        extractedObj := origObj.ExtractItems(fieldset.Leaves())
        // This is how the extractedObj looks like:
        // Carefully note that the required fields in 'ports' for 'merge' operation, ie port & protocol, are not there. And they should rightfully not be there.
        //extractedObj, err = objectType.FromUnstructured(jsonToInterface(`{"spec":{"ports":[{"nodePort":30001}]}}`))

        // If the extracted object looks like this, there won't be any error.
        // extractedObj, err = objectType.FromUnstructured(jsonToInterface(`{
        //    "spec": {
        //        "ports": [
        //            {
        //                "nodePort": 30001,
        //                "port": 81,
        //                "protocol": "TCP"
        //            }
        //        ]
        //    }
        //}`))
        // if err != nil {
        //  panic(err)
        // }
        logrus.Info("extracted items before merge", "extractedObj", JsonObjectToString(extractedObj.AsValue()))

        // Simulation: This is the new object which is created after having
        // merged with the first field manager 'kubectl-client-side-apply'.
        // We now want to merge the extracted fields from the second field
        // manager 'kubectl-edit'.
        newObj, err := objectType.FromUnstructured(jsonToInterface(`{"metadata":{"annotations":null},"spec":{"externalTrafficPolicy":"Cluster","internalTrafficPolicy":"Cluster","ports":[{"name":"http","port":80,"protocol":"TCP","targetPort":80}],"selector":{"app":"clear-nginx"},"sessionAffinity":"None","type":"NodePort"}}`))
        if err != nil {
            panic(err)
        }
        o, err := newObj.Merge(extractedObj)
        if err != nil {
            panic("failed to merge objects: " + err.Error())
        }
        // This returns error:
        // panic: failed to merge objects: .spec.ports: element 0: associative list with keys has an element that omits key field "port" (and doesn't have default value)
        logrus.Infof("%v", JsonObjectToString(o))
    }
}

// ParseableType constructs structured-merge-diff type from GVK.
func (r *Creator) ParseableType(ctx context.Context, gvk schema.GroupVersionKind) *typed.ParseableType {
    log := log.FromContext(ctx)

    typeName, ok := r.gvkToTypeNameMap[gvk]
    if !ok {
        return nil
    }
    log.V(1).Info("Model for GVK", "gvk", gvk, "typeName", typeName)
    return &typed.ParseableType{
        Schema:  r.schema,
        TypeRef: mergeDiffSchema.TypeRef{NamedType: &typeName},
    }
}

type Creator struct {
    restConfig       *rest.Config
    gvkToTypeNameMap map[schema.GroupVersionKind]string // Map from gvk to type name.
    schema           *mergeDiffSchema.Schema
}

func New(ctx context.Context, restConfig *rest.Config) (*Creator, error) {
    log := log.FromContext(ctx)

    dc := discovery.NewDiscoveryClientForConfigOrDie(restConfig)
    doc, err := dc.OpenAPISchema()
    if err != nil {
        return nil, err
    }
    models, err := proto.NewOpenAPIData(doc)
    if err != nil {
        return nil, err
    }
    typeSchema, err := schemaconv.ToSchemaWithPreserveUnknownFields(models, false)
    if err != nil {
        return nil, fmt.Errorf("failed to convert models to schema: %v", err)
    }

    creator := &Creator{
        restConfig:       restConfig,
        gvkToTypeNameMap: make(map[schema.GroupVersionKind]string),
        schema:           typeSchema,
    }

    // Construct map of GVK to type name. Parseable types expect type name together with schema.
    for _, modelName := range models.ListModels() {
        model := models.LookupModel(modelName)
        if model == nil {
            return nil, fmt.Errorf("ListModels returns a model that can't be looked-up for: %v", modelName)
        }
        gvkList := parseGroupVersionKind(model)
        for _, gvk := range gvkList {
            if len(gvk.Kind) > 0 {
                if existingModelName, ok := creator.gvkToTypeNameMap[gvk]; ok {
                    log.Info("duplicate GVK entry in OpenAPI schema", "gvk", gvk,
                        "modelName", modelName, "existingModelName", existingModelName)
                }
                creator.gvkToTypeNameMap[gvk] = modelName
            }
        }
    }

    return creator, nil
}

I understand the extracted field doesn't have the required fields. From extraction perspective, it shoudn't have it either. I think the fix needs to be there in the 'merge' part. Can someone help in understanding this?

0

There are 0 best solutions below