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

Bundle validate failing for webhooks due to missing spec.conversion.webhookClientConfig.service.port #3673

Closed
ahmedwaleedmalik opened this issue Aug 7, 2020 · 8 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. olm-integration Issue relates to the OLM integration
Milestone

Comments

@ahmedwaleedmalik
Copy link
Contributor

Bug Report

What did you do?

I followed the guide from the official documentation to add webhook. I have configured and set it up but when i try to generate bundle manifests. It fails at validation step with error:

Field spec.conversion.webhookClientConfig.service.port, Value 0: spec.conversion.webhookClientConfig.service.port: Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive

What did you see instead? Under which circumstances?

I am getting the following error:

/home/waleed/go//bin/controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests -q
/usr/bin/kustomize build config/manifests | operator-sdk generate bundle -q --overwrite --version 0.0.1  
INFO[0000] Building annotations.yaml                    
INFO[0000] Writing annotations.yaml in /home/waleed/go/src/github.com/stakater/jira-service-desk-operator/bundle/metadata 
INFO[0000] Building Dockerfile                          
INFO[0000] Writing bundle.Dockerfile in /home/waleed/go/src/github.com/stakater/jira-service-desk-operator 
operator-sdk bundle validate ./bundle
INFO[0000] Found annotations file                        bundle-dir=bundle container-tool=docker
INFO[0000] Could not find optional dependencies file     bundle-dir=bundle container-tool=docker
ERRO[0000] Error: Field spec.conversion.webhookClientConfig.service.port, Value 0: spec.conversion.webhookClientConfig.service.port: Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive

Environment

  • operator-sdk version:
    v0.19.0

  • go version:
    v1.14.4

  • Kubernetes version information:
    v1.17.3

  • Are you writing your operator in ansible, helm, or go?
    go

@camilamacedo86
Copy link
Contributor

Hi @estroz,

Is possible to integrate a project with OLM using webhooks? Do you know if webhooks are supported by OLM?

@camilamacedo86 camilamacedo86 added olm-integration Issue relates to the OLM integration triage/support Indicates an issue that is a support question. labels Aug 9, 2020
@robherley
Copy link

I'm also having this same issue, for now I added the missing fields:

spec:
  preserveUnknownFields: false # added this
  conversion:
    strategy: Webhook
    webhookClientConfig:
      caBundle: Cg==
      service:
        namespace: system
        name: webhook-service
        path: /convert
        port: 443 # added this, used 443 bc it's the default from the k8s docs
  • operator-sdk: v0.19.2
  • go: v1.14.5
  • k8s: v1.17.1
  • kustomize: v3.8.1

@estroz
Copy link
Member

estroz commented Aug 10, 2020

It looks like port is a required field in a CSV’s webhook description, which I imagine is because OLM reconstructs their configs before creating them. The webhook configs generated by make manifests do not have a service port so this port won’t be picked up by the CSV generator.

I think the right here to do is check if a webhook is configured to run from a service defined by a Service manifest in the generator’s input, which will be defined by by name, and if so use values from that manifest as CSV values. It may also be necessary to add the Service manifest to the bundle to be handled by OLM

@estroz
Copy link
Member

estroz commented Aug 10, 2020

Disregard what I said above, this appears to be a CRD validation issue. This may actually be a decoding bug in the validator's code: an uninitialized port value, intended to mean this value was not set in the manifest, should not cause this validation error, while an initialized but unset value should and will.

/cc @dinhxuanvu @kevinrizza

@awgreene
Copy link
Member

awgreene commented Aug 10, 2020

@camilamacedo86

Is possible to integrate a project with OLM using webhooks? Do you know if webhooks are supported by OLM?

Yes - OLM supports admission and conversion webhooks with a few constraints.

@awgreene
Copy link
Member

As @estroz suggested, this seems to be an issue with the validation library or the CRD itself. @ahmedwaleedmalik could you share the CRD you used which caused this issue?

@estroz estroz added kind/bug Categorizes issue or PR as related to a bug. and removed triage/support Indicates an issue that is a support question. labels Aug 10, 2020
@estroz estroz added this to the v1.1.0 milestone Aug 10, 2020
@estroz estroz self-assigned this Aug 10, 2020
@ahmedwaleedmalik
Copy link
Contributor Author

Hi @awgreene, it seems like an issue with generation of CRD rather the validation. Since, it is an auto-generated CRD we expect it to map spec.conversion.webhookClientConfig.service.port to what's defined in the service of that webhook. But it doesn't populate that field. Manually adding port fixes this issue.

The CRD that is missing port field:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: projects.jiraservicedesk.stakater.com
spec:
  preserveUnknownFields: false
  conversion:
    strategy: Webhook
    webhookClientConfig:
      # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank,
      # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager)
      caBundle: Cg==
      service:
        namespace: system
        name: webhook-service
        path: /convert

The resultant CRD that fails bundle validation:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  annotations:
    cert-manager.io/inject-ca-from: jira-service-desk-operator-system/jira-service-desk-operator-serving-cert
    controller-gen.kubebuilder.io/version: v0.3.0
  creationTimestamp: null
  name: projects.jiraservicedesk.stakater.com
spec:
  conversion:
    strategy: Webhook
    webhookClientConfig:
      caBundle: Cg==
      service:
        name: jira-service-desk-operator-webhook-service
        namespace: jira-service-desk-operator-system
        path: /convert
  group: jiraservicedesk.stakater.com
  names:
    kind: Project
    listKind: ProjectList
    plural: projects
    singular: project
  preserveUnknownFields: false
  scope: Cluster
  subresources:
    status: {}
  validation:
    openAPIV3Schema:
      description: Project is the Schema for the projects API
      properties:
        apiVersion:
          description: 'APIVersion defines the versioned schema of this representation
            of an object. 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
        kind:
          description: 'Kind is a string value representing the REST resource this
            object represents. Servers may infer this from the endpoint the client
            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
          type: string
        metadata:
          type: object
        spec:
          description: ProjectSpec defines the desired state of Project
          properties:
            assigneeType:
              description: Task assignee type
              enum:
              - PROJECT_LEAD
              - UNASSIGNED
              type: string
            avatarId:
              description: An integer value for the project's avatar.
              type: integer
            categoryId:
              description: The ID of the project's category
              type: integer
            description:
              description: Description for project
              type: string
            issueSecurityScheme:
              description: The ID of the issue security scheme for the project, which
                enables you to control who can and cannot view issues
              type: integer
            key:
              description: The project key is used as the prefix of your project's
                issue keys
              type: string
            leadAccountId:
              description: ID of project lead
              maxLength: 128
              type: string
            name:
              description: Name of the project
              type: string
            notificationScheme:
              description: The ID of the notification scheme for the project
              type: integer
            permissionScheme:
              description: The ID of the permission scheme for the project
              type: integer
            projectTemplateKey:
              description: A prebuilt configuration for a project
              type: string
            projectTypeKey:
              description: The project type, which dictates the application-specific
                feature set
              enum:
              - business
              - service_desk
              - software
              type: string
            url:
              description: A link to information about this project, such as project
                documentation
              type: string
          type: object
        status:
          description: ProjectStatus defines the observed state of Project
          properties:
            conditions:
              description: Status conditions
              items:
                description: "Condition represents an observation of an object's state.
                  Conditions are an extension mechanism intended to be used when the
                  details of an observation are not a priori known or would not apply
                  to all instances of a given Kind. \n Conditions should be added
                  to explicitly convey properties that users and components care about
                  rather than requiring those properties to be inferred from other
                  observations. Once defined, the meaning of a Condition can not be
                  changed arbitrarily - it becomes part of the API, and has the same
                  backwards- and forwards-compatibility concerns of any other part
                  of the API."
                properties:
                  lastTransitionTime:
                    format: date-time
                    type: string
                  message:
                    type: string
                  reason:
                    description: ConditionReason is intended to be a one-word, CamelCase
                      representation of the category of cause of the current status.
                      It is intended to be used in concise output, such as one-line
                      kubectl get output, and in summarizing occurrences of causes.
                    type: string
                  status:
                    type: string
                  type:
                    description: "ConditionType is the type of the condition and is
                      typically a CamelCased word or short phrase. \n Condition types
                      should indicate state in the \"abnormal-true\" polarity. For
                      example, if the condition indicates when a policy is invalid,
                      the \"is valid\" case is probably the norm, so the condition
                      should be called \"Invalid\"."
                    type: string
                required:
                - status
                - type
                type: object
              type: array
            id:
              description: Jira service desk project ID
              type: string
          required:
          - id
          type: object
      type: object
  version: v1alpha1
  versions:
  - name: v1alpha1
    served: true
    storage: true
status:
  acceptedNames:
    kind: ""
    plural: ""
  conditions: []
  storedVersions: []

This is open source and the port fix was done against this PR: stakater/jira-service-desk-operator#10

@ahmedwaleedmalik
Copy link
Contributor Author

Closing this since i couldn't reproduce this issue in v1.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. olm-integration Issue relates to the OLM integration
Projects
None yet
Development

No branches or pull requests

5 participants