Skip to content

Conversation

@awgreene
Copy link
Member

This commit introduces documentation that defines OLM support for
Validating and Mutating Admission Webhooks.

Copy link
Member

@anik120 anik120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@awgreene Thank you for your PR. I've left some comments on organisation of the doc. Specifically, the way the hugo doc site is laid out, we want to organize things a little differently than our docs in our github repo. After you open your PR, you can check the netlify/operator-lifecycle-manager/deploy-preview target that CI provides for you, to get a preview of what the doc site will look like with your current changes.

Looks like since this doc does not have the header that hugo expects, it's not showing up in the doc site as of now.

Also, it feels like Adding Admission Webhook should be under the Advanced task section.

We organised the Core task section to include a logical sequence of tasks you can perform with OLM:

  • Install OLM
  • Make your operator part of a Catalog
  • Create a Catsrc with the Catalog
  • Create a subscription to your operator
    .
    .
    etc.

Seems like adding admission webhook does not go in the flow of core tasks for now. Please correct me if I'm wrong.

@awgreene awgreene force-pushed the admission-webhooks-doc branch 9 times, most recently from ef40030 to 9dec818 Compare April 29, 2020 23:59
@awgreene
Copy link
Member Author

@anik120 I think we're good to go, can you review?

Copy link
Member

@anik120 anik120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@awgreene Thank you for making the changes. Looks like we have the doc in the right place and it renders in the website correctly. I've made a few suggestions around the language in the doc, just so that the language in the doc is consistent with the other docs.

Otherwise /approve

@anik120 anik120 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 30, 2020
@awgreene awgreene force-pushed the admission-webhooks-doc branch from 9dec818 to 60901a7 Compare April 30, 2020 14:57
@awgreene
Copy link
Member Author

Great edits @anik120

@awgreene awgreene force-pushed the admission-webhooks-doc branch 2 times, most recently from 23de1b4 to d274e7c Compare May 5, 2020 17:54
Copy link
Member

@njhale njhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome! I have a few last comments that I'd like to get your thoughts on before we merge.

@awgreene awgreene force-pushed the admission-webhooks-doc branch from d274e7c to 57d4164 Compare May 7, 2020 13:12
This commit introduces documentation that defines OLM support for
Validating and Mutating Admission Webhooks.
@awgreene awgreene force-pushed the admission-webhooks-doc branch from 57d4164 to 584089b Compare May 7, 2020 19:04
Copy link
Member

@njhale njhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 7, 2020
@awgreene awgreene merged commit a07edb0 into operator-framework:master May 7, 2020
@beekhof
Copy link

beekhof commented May 18, 2020

When did/will OLM start supporting this?

@awgreene
Copy link
Member Author

@beekhof Webhook support was merged in this PR, which will be featured in OpenShift 4.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants