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

Discussion: How to add CLI meta data to Source CRDs ? #1940

Closed
rhuss opened this issue Sep 20, 2019 · 12 comments
Closed

Discussion: How to add CLI meta data to Source CRDs ? #1940

rhuss opened this issue Sep 20, 2019 · 12 comments
Labels
area/discovery area/sources kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Milestone

Comments

@rhuss
Copy link
Contributor

rhuss commented Sep 20, 2019

Problem

For helping kn (or any other CLI) it would be very helpful when a Source CRD would carry some extra meta information for mapping cli-options (like --access-token or --event-types for a GitHubSource) to the property within the CR (e.g. .spec.accessToken.secreteKeyRef for --access-token and .spect.eventTypes for --event-types).

Fortunately, OpenAPI schema supports so-called Specification Extensions in a Schema Object. These are fields starting with a x-

So ideally a GitHubSource CRD could look like in the example below (where I picked up the existing GitHub source, and added the required meta information to the properties which are supposed to be mapped. I also moved the registry types away from the schema to annotations, but that's not important here)

GitHub Source with `x-cli-` annotations
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: githubsources.sources.eventing.knative.dev
  labels:
    contrib.eventing.knative.dev/release: devel
    eventing.knative.dev/source: "true"
    knative.dev/crd-install: "true"
  annotations:
    registry.knative.dev/check_suite.type: dev.knative.source.github.check_suite
    registry.knative.dev/commit_comment.type: dev.knative.source.github.commit_comment
    registry.knative.dev/create.type: dev.knative.source.github.create
    registry.knative.dev/delete.type: dev.knative.source.github.delete
    registry.knative.dev/deployment.type: dev.knative.source.github.deployment
    registry.knative.dev/deployment_status.type: dev.knative.source.github.deployment_status
    registry.knative.dev/fork.type: dev.knative.source.github.fork
    registry.knative.dev/gollum.type: dev.knative.source.github.gollum
    registry.knative.dev/installation.type: dev.knative.source.github.installation
    registry.knative.dev/integration_installation.type: dev.knative.source.github.integration_installation
    registry.knative.dev/issue_comment.type: dev.knative.source.github.issue_comment
    registry.knative.dev/issues.type: dev.knative.source.github.issues
    registry.knative.dev/member.type: dev.knative.source.github.member
    registry.knative.dev/milestone.type: dev.knative.source.github.milestone
    registry.knative.dev/org_block.type: dev.knative.source.github.org_block
    registry.knative.dev/ping.type: dev.knative.source.github.ping
    registry.knative.dev/project_column.type: dev.knative.source.github.project_column
    registry.knative.dev/public.type: dev.knative.source.github.public
    registry.knative.dev/pull_request_review.type: dev.knative.source.github.pull_request_review
    registry.knative.dev/push.type: dev.knative.source.github.push
    registry.knative.dev/repository.type: dev.knative.source.github.repository
    registry.knative.dev/team.type: dev.knative.source.github.team
    registry.knative.dev/watch.type: dev.knative.source.github.watch
spec:
  group: sources.eventing.knative.dev
  names:
    categories:
    - all
    - knative
    - eventing
    - sources
    kind: GitHubSource
    plural: githubsources
  scope: Namespaced
  subresources:
    status: {}
  validation:
    openAPIV3Schema:
      properties:
        spec:
          properties:
            accessToken:
              properties:
                secretKeyRef:
                  # Name of a cli option mapping to this field
                  x-cli-option: "access-token"
                  # Type which triggers a special treatment of the option to
                  # create the embedded opject with a `key:` and `name:` field
                  x-cli-type: "map-selector"
                  # Help message which can be used by a CLI on the help page
                  x-cli-help: "GitHub access token. The value of this option is the name of a secret and key within secret to GitHub access, dot separated (e.g. --secret-token=mygithubsecret.mykey) "
                  type: object
              type: object
            eventTypes:
              items:
                enum:
                - check_suite
                - commit_comment
                - create
                - delete
                - deployment
                - deployment_status
                - fork
                - gollum
                - installation
                - integration_installation
                - issue_comment
                - issues
                - label
                - member
                - membership
                - milestone
                - organization
                - org_block
                - page_build
                - ping
                - project_card
                - project_column
                - project
                - public
                - pull_request
                - pull_request_review
                - pull_request_review_comment
                - push
                - release
                - repository
                - status
                - team
                - team_add
                - watch
                type: string
              minItems: 1
              x-cli-option: "event-type"
              x-cli-help: "GitHub events to watch for. This is a comma separated list of options. Alternatively this option can be provided multiple times."
              type: array
            ownerAndRepository:
              x-cli-option: "owner"
              minLength: 1
              type: string
            secretToken:
              properties:
                secretKeyRef:
                  x-cli-option: "secret-token"
                  x-cli-type: "map-selector"
                  x-cli-help: "GitHub secret token. The value of this option is the name of a secret and key within secret to GitHub access, dot separated (e.g. --secret-token=mygithubsecret.mykey) "
                  type: object
              type: object
            serviceAccountName:
              x-cli-option: "service-account"
              x-cli-help: "Service account"
              type: string
            sink:
              type: object
          required:
          - ownerAndRepository
          - eventTypes
          - accessToken
          - secretToken
          type: object
        status:
          properties:
            conditions:
              items:
                properties:
                  lastTransitionTime:
                    # we use a string in the stored object but a wrapper object
                    # at runtime.
                    type: string
                  message:
                    type: string
                  reason:
                    type: string
                  severity:
                    type: string
                  status:
                    type: string
                  type:
                    type: string
                required:
                - type
                - status
                type: object
              type: array
            sinkUri:
              type: string
            webhookIDKey:
              type: string
          type: object
  version: v1alpha1

Unfortunately, Kubernetes does not support this (see kubernetes/kubernetes#82942 and also the API docs for CRDs where it seems only a fixed set of x-kubernetes- fields are supported

😞

The question is how to proceed:

  • Wait if Kubernetes fix this ? (if even the issue is not rejected with wontfix)
  • Creatively "reuse" one of the existing fields of the OpenAPI Schema Object Spec, like
  • example: "A free-form property to include an example of an instance for this schema. To represent examples that cannot be naturally represented in JSON or YAML, a string value can be used to contain the example with escaping where necessary."
  • externalDocs : An object with description and url field

An solution would be also to put the mapping into CRD annotations with the cli-option as key and the JSON path expression as value (or having a single annotation which has a JSON mapping as value), e.g

annotations:
  client.knative.dev/access-token.option=.spec.accessToken.secretRef
  client.knative.dev/access-token.help="GitHub access token. The value of this option is the name of a secret and key within secret to GitHub access, dot separated (e.g. --secret-token=mygithubsecret.mykey)"
  client.knative.dev/access-token.type="map-selector"

Tbh, I would prefer a hook into the schema as it feels that it belongs to the schema as an 'annotation' kind.

What do you think about the proposal or is there any other way to help the client to create CRs from flat command line options ?

@aaron-lerner
Copy link

An issue I see with this is that I don't think all CLIs will be able to be able to dynamically add flags for each x-cli-option.

@rhuss
Copy link
Contributor Author

rhuss commented Sep 27, 2019

Yes, but they alway could ;-).

This meta-data is helpful for CLI authors if they want to support dynamic cli options, based on the CR at hand. This is supported by any language, but it could be that your favourite options parsing library doesn't support this. However, you can still do the CLI options parsing manually, this is not a principal limitation for a client.

@rhuss
Copy link
Contributor Author

rhuss commented Sep 27, 2019

Following the lines of the discussion starting at #1550 (comment) and the fact that we won't see support for arbitrary x-cli- schema fields in K8s, I suggest the following approach:

We introduce in the Source Schema Spec the support of an annotation client.knative.dev/options which takes the form:

annotations:
  client.knative.dev/options: |
    [
      {
       "name" : "access-token",
       "path" : ".spec.accessToken.secretRef",
       "help" : "GitHub access token. The value of this option is the name of a secret and key within secret to GitHub access, dot separated (e.g. --access-token=mygithubsecret.mykey)"
       "type" : "secret-reference"
      },
      {
       "name" : "event-type",
       "path" : ".spec.eventTypes",
       "help" : "GitHub event type. This option can be given multiple times for registering multiple event types"
      },
      ....
    ]

@rhuss
Copy link
Contributor Author

rhuss commented Sep 27, 2019

@adamharwayne if this sounds ok, I would add this to the "Kn eventing" proposal, too.

@aslom
Copy link
Member

aslom commented Oct 1, 2019

This is great idea to make CLI easier to use.

@vaikas
Copy link
Contributor

vaikas commented Oct 3, 2019

We should also add this to the Source Spec after it lands, I don't want to hold off on that before merging, but once it lands on the spec, we can close this?
#1856

@vaikas vaikas modified the milestones: v0.10.0-M2, v0.11.0-M1 Oct 3, 2019
@vaikas
Copy link
Contributor

vaikas commented Oct 3, 2019

I'd like to get the Source Spec changes in for v0.10.0-M2 and finish the CRDs to support this with little more leeway, if we can get everything done for 10-M2, awesome :)

@n3wscott
Copy link
Contributor

YEAH!!! WOOO with the Manual CRD via Discovery API.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 26, 2020
@rhuss
Copy link
Contributor Author

rhuss commented Jan 13, 2021

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 13, 2021
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2021
@rhuss
Copy link
Contributor Author

rhuss commented Apr 14, 2021

Let's close this as it seems to going into the direction to leverage the Discovery API to hold those CLI related meta-data, so let's continue as part of the conversation how to integrate this meta-data into the Source type

@rhuss rhuss closed this as completed Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/discovery area/sources kind/feature-request lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

8 participants