Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling Module CRD Upgrade Regarding Conversion Webhook Setup #941

Open
ruanxin opened this issue Oct 1, 2024 · 1 comment
Open

Handling Module CRD Upgrade Regarding Conversion Webhook Setup #941

ruanxin opened this issue Oct 1, 2024 · 1 comment
Labels
decision Related to all issues that need a decision

Comments

@ruanxin
Copy link
Contributor

ruanxin commented Oct 1, 2024

Created on 2024-10-01 by Xin Ruan (@ruanxin).

Decision log

Name Description
Title Handling CRD Upgrade Regarding Conversion Webhook Setup
Due date 2024-10-30
Status Accepted
Decision type Binary
Affected decisions None

Context

The CustomResourceDefinition (CRD) of module Custom Resources (CRs) is deployed and managed by the Kyma Lifecycle Manager (KLM). However, the spec.conversion.webhook.caBound field is not offered by KLM and must be dynamically configured by the module manager in the SKR runtime during provisioning time. To avoid conflicts between the KLM and the module manager, it is necessary to define field management responsibilities clearly.

Currently, some modules already configure the conversion webhook, where the module manager only patches spec.conversion.webhook.caBound, but the rest of the fields in spec.conversion are declared in the submitted CRD. This approach works now, but if the module decides to configure spec.conversion.strategy to None, the API server might handle this merge due to overlapping field managers declared by KLM and the module manager, resulting in errors such as:

The CustomResourceDefinition is invalid: spec.conversion.webhookClientConfig: Forbidden: should not be set when strategy is not set to Webhook

Therefore, a clear guideline and migration path are needed.

Decision

The decision is that:

  • Module Manager Responsibility: The Module Manager should handle all spec.conversion sections during provisioning time. This ensures that the dynamic configuration of the conversion webhook is managed consistently and without conflicts.

  • CRD Submission: The CRD submitted by the module team should not contain the spec.conversion section. This prevents KLM from managing this field and avoids potential conflicts during API upgrade.

  • Validation in Submission Pipeline: The module submission pipeline should include a validation step to ensure that future module CRDs do not contain the spec.conversion section. This validation step will enforce the guideline and prevent accidental inclusion of the spec.conversion section in submitted CRDs.

Guidelines

For module haven't configured conversion webhook

  1. Module Submission:

    • Ensure that the CRD submitted in the future does not contain the spec.conversion section.
  2. Provisioning Time:

    • When the Module requires to configure conversion webhook, the Module Manager should dynamically patch the spec.conversion section during provisioning as well as maintains the consistency of this field.
    • Example configuration:
      spec:
        conversion:
          strategy: Webhook
          webhook:
            clientConfig:
              service:
                name: myresource-webhook-service
                namespace: default
                path: /convert
            conversionReviewVersions: ["v1", "v1beta1"]
            caBound: <dynamically-configured-ca>

For existing module have configured conversion webhook

To resolve potential problems mentioned in the context that might occur during the next CRD upgrade when trying to remove spec.conversion, the following manual process must be implemented so that the next version upgrade will be aligned with the rest of the modules.

  1. Remove module manager managedField before the next Module CRD upgrade:
    Check the CRD deployed in the SKR cluster and identify your module manager's managedFields declared by the API Server. In the example below, the module manager is in position 2.

    Note that this position might vary in different SKRs, so a detection process to identify the correct position is necessary.

      managedFields:
     - manager: declarative.kyma-project.io/applier
       ...
     - manager: kube-apiserver
       ...
     - manager: manager
       operation: Update
       apiVersion: apiextensions.k8s.io/v1
       time: '2024-09-17T11:55:26Z'
       fieldsType: FieldsV1
       fieldsV1:
         f:spec:
           f:conversion:
             f:webhook:
               f:clientConfig:
                 f:caBundle: {}
    
    • Use a client patch to remove the entry of your module manager in the CRD. For example, with kubectl, you can use the following command:
      kubectl patch crd <crd-name> --type=json -p='[{"op": "remove", "path": "/metadata/managedFields/[your manager position]"}]'
  2. Follow the normal guideline for the next CRD upgrade:

    • After confirming that the managedField has been removed, the module team can follow the normal guideline for the next CRD upgrade.

Consequences

Positive:

  • Clear separation of responsibilities between KLM and the Module Manager.
  • Avoid conflicts during module CR API upgrade.

Negative:

  • Requires coordination and updates to existing CRDs.
@ruanxin ruanxin added the decision Related to all issues that need a decision label Oct 1, 2024
@ruanxin
Copy link
Contributor Author

ruanxin commented Oct 12, 2024

For Validation in Submission Pipeline: https://github.tools.sap/kyma/test-infra/issues/453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision Related to all issues that need a decision
Projects
None yet
Development

No branches or pull requests

1 participant