📖 catalogd API Reference Generation #1240
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Makefile
Outdated
| --config=$(ROOT_DIR)/docs/refs/api/crd-ref-docs-gen-config.yaml \ | ||
| --renderer=markdown --output-path=$(ROOT_DIR)/docs/refs/api/$(OPERATOR_CONTROLLER_API_REFERENCE_FILENAME); | ||
| rm -f $(ROOT_DIR)/docs/refs/api/$(CATALOGD_API_REFERENCE_FILENAME) | ||
| $(CRD_REF_DOCS) --source-path=$(ROOT_DIR)/vendor/github.com/operator-framework/catalogd/api \ |
There was a problem hiding this comment.
Are we OK with grabbing this from vendor? I feel like there has to be a better option but nothing else obvious occurred to me.
There was a problem hiding this comment.
We don't vendor our dependencies in this repo, so I think we need to do something different.
However, there is a plan afoot to make a monorepo out of operator-controller and catalogd. If there isn't an interim solution for generating docs from out-of-tree APIs, we'll be able to do this after we monorepo-ize.
There was a problem hiding this comment.
Another thing we can do, which is a little less user friendly/environment independent, is just assume that catalogd is checked out in ../catalogd. That works fine with the verify github action, but might bother some devs if they have a different directory structure.
There was a problem hiding this comment.
@joelanford Not having thought of anything else, I've updated this to do above. Let me know if you're alright with this method. It's definitely a little clumsy though, since a local environment isn't guaranteed to have catalogd checked out in the right spot.
There was a problem hiding this comment.
@dtfranz What do you think about using git to clone the version of catalogd we depend on in go.mod into a temporary directory and pointing to that instead?
I saw that you were able to clone catalogd in CI to the appropriate path, but I'd like to see it work without any additional configuration on the dev side.
That being said, this probably doesn't really matter that much since we are actively planning to move to a monorepo (already seems to be good consensus on the monorepo idea, now we are hashing out the how). Because of this I won't block the PR on this and will leave it up to you or others to make that decision.
There was a problem hiding this comment.
Thanks @everettraven ! That's a good call. I'll merge this for now and do that as an immediate follow-up. As a bonus, it would simplify the github action a little bit as well.
| --config=$(ROOT_DIR)/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml \ | ||
| --renderer=markdown \ | ||
| --output-path=$(ROOT_DIR)/docs/refs/api/$(API_REFERENCE_FILENAME) | ||
| --config=$(ROOT_DIR)/docs/refs/api/crd-ref-docs-gen-config.yaml \ |
There was a problem hiding this comment.
I'd misunderstood the usage of this flag when I first implemented this, but turns out if you don't specify --source-path and do specify --config as your CRD file, it generates the same doc. Confusing? Or maybe it finds the api folder automatically if not specified?
Regardless, this is the actual intended usage. --config specifies a configuration file for the generator. At the moment we just specify the reference k8s version, which also consequently fixes the broken k8s API reference links in our doc.
There was a problem hiding this comment.
Non-blocking meta question:
Is there a way to specify a CRD instead? I ask because I know there have been cases in the past (and maybe present?) where we patch the CRD via kustomize after generating it from Go code.
If we generate from Go code, the docs don't account for those patches.
There was a problem hiding this comment.
From what I can tell, no, the api/ folder is the only available source of truth for this tool. If the --source-path isn't specified, it'll crawl through your folder structure and find the api/ folder itself.
Do you have an example of what kind of change we would make from a patch like that? Something like field descriptions, validations, or other things not required to be present in the _types.go file is what I was thinking of. We may be able to cover this kind of thing using their custom markers and templates (reference).
There was a problem hiding this comment.
Here's an example from catalogd: https://github.com/operator-framework/catalogd/blob/main/config/base/crd/patches/catalog_validation.yaml
There was a problem hiding this comment.
@joelanford I think we should ensure that we are appropriately tracking any and all validations, even if they are kustomize patches, in the GoDoc of the field(s) associated with it. In the example from catalogd, I would expect the GoDoc in the ClusterCatalog.Spec.Source type to reflect that the image field is required if the source type is set to image
| @@ -81,7 +81,7 @@ _Appears in:_ | |||
| | `packageName` _string_ | packageName is a reference to the name of the package to be installed<br />and is used to filter the content from catalogs.<br /><br />This field is required, immutable and follows the DNS subdomain name<br />standard as defined in [RFC 1123]. This means that valid entries:<br /> - Contain no more than 253 characters<br /> - Contain only lowercase alphanumeric characters, '-', or '.'<br /> - Start with an alphanumeric character<br /> - End with an alphanumeric character<br /><br />Some examples of valid values are:<br /> - some-package<br /> - 123-package<br /> - 1-package-2<br /> - somepackage<br /><br />Some examples of invalid values are:<br /> - -some-package<br /> - some-package-<br /> - thisisareallylongpackagenamethatisgreaterthanthemaximumlength<br /> - some.package<br /><br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 253 <br />Pattern: `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` <br /> | | |||
| | `version` _string_ | version is an optional semver constraint (a specific version or range of versions). When unspecified, the latest version available will be installed.<br /><br />Acceptable version ranges are no longer than 64 characters.<br />Version ranges are composed of comma- or space-delimited values and one or<br />more comparison operators, known as comparison strings. Additional<br />comparison strings can be added using the OR operator (\|\|).<br /><br /># Range Comparisons<br /><br />To specify a version range, you can use a comparison string like ">=3.0,<br /><3.6". When specifying a range, automatic updates will occur within that<br />range. The example comparison string means "install any version greater than<br />or equal to 3.0.0 but less than 3.6.0.". It also states intent that if any<br />upgrades are available within the version range after initial installation,<br />those upgrades should be automatically performed.<br /><br /># Pinned Versions<br /><br />To specify an exact version to install you can use a version range that<br />"pins" to a specific version. When pinning to a specific version, no<br />automatic updates will occur. An example of a pinned version range is<br />"0.6.0", which means "only install version 0.6.0 and never<br />upgrade from this version".<br /><br /># Basic Comparison Operators<br /><br />The basic comparison operators and their meanings are:<br /> - "=", equal (not aliased to an operator)<br /> - "!=", not equal<br /> - "<", less than<br /> - ">", greater than<br /> - ">=", greater than OR equal to<br /> - "<=", less than OR equal to<br /><br /># Wildcard Comparisons<br /><br />You can use the "x", "X", and "*" characters as wildcard characters in all<br />comparison operations. Some examples of using the wildcard characters:<br /> - "1.2.x", "1.2.X", and "1.2.*" is equivalent to ">=1.2.0, < 1.3.0"<br /> - ">= 1.2.x", ">= 1.2.X", and ">= 1.2.*" is equivalent to ">= 1.2.0"<br /> - "<= 2.x", "<= 2.X", and "<= 2.*" is equivalent to "< 3"<br /> - "x", "X", and "*" is equivalent to ">= 0.0.0"<br /><br /># Patch Release Comparisons<br /><br />When you want to specify a minor version up to the next major version you<br />can use the "~" character to perform patch comparisons. Some examples:<br /> - "~1.2.3" is equivalent to ">=1.2.3, <1.3.0"<br /> - "~1" and "~1.x" is equivalent to ">=1, <2"<br /> - "~2.3" is equivalent to ">=2.3, <2.4"<br /> - "~1.2.x" is equivalent to ">=1.2.0, <1.3.0"<br /><br /># Major Release Comparisons<br /><br />You can use the "^" character to make major release comparisons after a<br />stable 1.0.0 version is published. If there is no stable version published, // minor versions define the stability level. Some examples:<br /> - "^1.2.3" is equivalent to ">=1.2.3, <2.0.0"<br /> - "^1.2.x" is equivalent to ">=1.2.0, <2.0.0"<br /> - "^2.3" is equivalent to ">=2.3, <3"<br /> - "^2.x" is equivalent to ">=2.0.0, <3"<br /> - "^0.2.3" is equivalent to ">=0.2.3, <0.3.0"<br /> - "^0.2" is equivalent to ">=0.2.0, <0.3.0"<br /> - "^0.0.3" is equvalent to ">=0.0.3, <0.0.4"<br /> - "^0.0" is equivalent to ">=0.0.0, <0.1.0"<br /> - "^0" is equivalent to ">=0.0.0, <1.0.0"<br /><br /># OR Comparisons<br />You can use the "\|\|" character to represent an OR operation in the version<br />range. Some examples:<br /> - ">=1.2.3, <2.0.0 \|\| >3.0.0"<br /> - "^0 \|\| ^3 \|\| ^5"<br /><br />For more information on semver, please see https://semver.org/ | | MaxLength: 64 <br />Pattern: `^(\s*(=\|\|!=\|>\|<\|>=\|=>\|<=\|=<\|~\|~>\|\^)\s*(v?(0\|[1-9]\d*\|[x\|X\|\*])(\.(0\|[1-9]\d*\|x\|X\|\*]))?(\.(0\|[1-9]\d*\|x\|X\|\*))?(-([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?(\+([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?)\s*)((?:\s+\|,\s*\|\s*\\|\\|\s*)(=\|\|!=\|>\|<\|>=\|=>\|<=\|=<\|~\|~>\|\^)\s*(v?(0\|[1-9]\d*\|x\|X\|\*])(\.(0\|[1-9]\d*\|x\|X\|\*))?(\.(0\|[1-9]\d*\|x\|X\|\*]))?(-([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?(\+([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?)\s*)*$` <br /> | | |||
| | `channels` _string array_ | channels is an optional reference to a set of channels belonging to<br />the package specified in the packageName field.<br /><br />A "channel" is a package author defined stream of updates for an extension.<br /><br />When specified, it is used to constrain the set of installable bundles and<br />the automated upgrade path. This constraint is an AND operation with the<br />version field. For example:<br /> - Given channel is set to "foo"<br /> - Given version is set to ">=1.0.0, <1.5.0"<br /> - Only bundles that exist in channel "foo" AND satisfy the version range comparison will be considered installable<br /> - Automatic upgrades will be constrained to upgrade edges defined by the selected channel<br /><br />When unspecified, upgrade edges across all channels will be used to identify valid automatic upgrade paths.<br /><br />This field follows the DNS subdomain name standard as defined in [RFC<br />1123]. This means that valid entries:<br /> - Contain no more than 253 characters<br /> - Contain only lowercase alphanumeric characters, '-', or '.'<br /> - Start with an alphanumeric character<br /> - End with an alphanumeric character<br /><br />Some examples of valid values are:<br /> - 1.1.x<br /> - alpha<br /> - stable<br /> - stable-v1<br /> - v1-stable<br /> - dev-preview<br /> - preview<br /> - community<br /><br />Some examples of invalid values are:<br /> - -some-channel<br /> - some-channel-<br /> - thisisareallylongchannelnamethatisgreaterthanthemaximumlength<br /> - original_40<br /> - --default-channel<br /><br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | | | |||
| | `selector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#labelselector-v1-meta)_ | selector is an optional field that can be used<br />to filter the set of ClusterCatalogs used in the bundle<br />selection process.<br /><br />When unspecified, all ClusterCatalogs will be used in<br />the bundle selection process. | | | | |||
| | `selector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | selector is an optional field that can be used<br />to filter the set of ClusterCatalogs used in the bundle<br />selection process.<br /><br />When unspecified, all ClusterCatalogs will be used in<br />the bundle selection process. | | | | |||
There was a problem hiding this comment.
As noted in earlier comment, these links were broken due to incorrect config file usage.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1240 +/- ##
=======================================
Coverage 76.18% 76.18%
=======================================
Files 40 40
Lines 2330 2330
=======================================
Hits 1775 1775
Misses 398 398
Partials 157 157
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dtfranz
force-pushed
the
catalogd-api-reference-docs
branch
3 times, most recently
from
718ee99
to
8a47f71
Compare
openshift-merge-robot
added
the
needs-rebase
dtfranz
force-pushed
the
catalogd-api-reference-docs
branch
from
8a47f71
to
72abbde
Compare
openshift-merge-robot
removed
the
needs-rebase
dtfranz
force-pushed
the
catalogd-api-reference-docs
branch
7 times, most recently
from
3cc8292
to
abcfa9a
Compare
openshift-merge-robot
added
the
needs-rebase
dtfranz
force-pushed
the
catalogd-api-reference-docs
branch
from
abcfa9a
to
a47a316
Compare
openshift-merge-robot
removed
the
needs-rebase
Generates the catalogd API reference along with the operator-controller's Signed-off-by: dtfranz <dfranz@redhat.com>
dtfranz
force-pushed
the
catalogd-api-reference-docs
branch
from
a47a316
to
390bbc3
Compare
Generates the catalogd API reference along with the operator-controller's
Closes #1125
Description
Reviewer Checklist