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

[bitnami/kubeapps] Add nullable annotation in a param #9209

Merged
merged 6 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bitnami/kubeapps/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ maintainers:
name: kubeapps
sources:
- https://github.com/kubeapps/kubeapps
version: 7.8.2
version: 7.8.3
26 changes: 13 additions & 13 deletions bitnami/kubeapps/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Kubeapps is a web-based UI for launching and managing applications on Kubernetes
[Overview of Kubeapps](https://kubeapps.com)



## TL;DR

```bash
Expand All @@ -28,7 +28,7 @@ With Kubeapps you can:

- Customize deployments through an intuitive, form-based user interface
- Inspect, upgrade and delete applications installed in the cluster
- Browse and deploy [Helm](https://github.com/helm/helm) charts from public or private chart repositories (including [VMware Marketplace���������](https://marketplace.cloud.vmware.com) and [Bitnami Application Catalog](https://bitnami.com/application-catalog))
- Browse and deploy [Helm](https://github.com/helm/helm) charts from public or private chart repositories (including [VMware Marketplace](https://marketplace.cloud.vmware.com) and [Bitnami Application Catalog](https://bitnami.com/application-catalog))
- Browse and deploy [Kubernetes Operators](https://operatorhub.io/)
- Secure authentication to Kubeapps using a [standalone OAuth2/OIDC provider](https://github.com/kubeapps/kubeapps/blob/main/docs/user/using-an-OIDC-provider.md) or [using Pinniped](https://github.com/kubeapps/kubeapps/blob/main/docs/user/using-an-OIDC-provider-with-pinniped.md)
- Secure authorization based on Kubernetes [Role-Based Access Control](https://github.com/kubeapps/kubeapps/blob/main/docs/user/access-control.md)
Expand Down Expand Up @@ -117,7 +117,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| ------------------------------------------------ | ----------------------------------------------------------------------------------------- | ---------------------- |
| `frontend.image.registry` | NGINX image registry | `docker.io` |
| `frontend.image.repository` | NGINX image repository | `bitnami/nginx` |
| `frontend.image.tag` | NGINX image tag (immutable tags are recommended) | `1.21.6-debian-10-r21` |
| `frontend.image.tag` | NGINX image tag (immutable tags are recommended) | `1.21.6-debian-10-r27` |
| `frontend.image.pullPolicy` | NGINX image pull policy | `IfNotPresent` |
| `frontend.image.pullSecrets` | NGINX image pull secrets | `[]` |
| `frontend.image.debug` | Enable image debug mode | `false` |
Expand Down Expand Up @@ -185,7 +185,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| ------------------------------------------------- | -------------------------------------------------------------------------------------------- | ---------------------------- |
| `dashboard.image.registry` | Dashboard image registry | `docker.io` |
| `dashboard.image.repository` | Dashboard image repository | `bitnami/kubeapps-dashboard` |
| `dashboard.image.tag` | Dashboard image tag (immutable tags are recommended) | `2.4.3-debian-10-r0` |
| `dashboard.image.tag` | Dashboard image tag (immutable tags are recommended) | `2.4.3-debian-10-r6` |
| `dashboard.image.pullPolicy` | Dashboard image pull policy | `IfNotPresent` |
| `dashboard.image.pullSecrets` | Dashboard image pull secrets | `[]` |
| `dashboard.image.debug` | Enable image debug mode | `false` |
Expand Down Expand Up @@ -256,7 +256,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `apprepository.image.pullSecrets` | Kubeapps AppRepository Controller image pull secrets | `[]` |
| `apprepository.syncImage.registry` | Kubeapps Asset Syncer image registry | `docker.io` |
| `apprepository.syncImage.repository` | Kubeapps Asset Syncer image repository | `bitnami/kubeapps-asset-syncer` |
| `apprepository.syncImage.tag` | Kubeapps Asset Syncer image tag (immutable tags are recommended) | `2.4.3-scratch-r1` |
| `apprepository.syncImage.tag` | Kubeapps Asset Syncer image tag (immutable tags are recommended) | `2.4.3-scratch-r2` |
| `apprepository.syncImage.pullPolicy` | Kubeapps Asset Syncer image pull policy | `IfNotPresent` |
| `apprepository.syncImage.pullSecrets` | Kubeapps Asset Syncer image pull secrets | `[]` |
| `apprepository.globalReposNamespaceSuffix` | Suffix for the namespace of global repos. Defaults to empty for backwards compatibility. | `""` |
Expand Down Expand Up @@ -301,7 +301,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `kubeops.enabled` | Specifies whether this component should be installed. | `true` |
| `kubeops.image.registry` | Kubeops image registry | `docker.io` |
| `kubeops.image.repository` | Kubeops image repository | `bitnami/kubeapps-kubeops` |
| `kubeops.image.tag` | Kubeops image tag (immutable tags are recommended) | `2.4.3-scratch-r1` |
| `kubeops.image.tag` | Kubeops image tag (immutable tags are recommended) | `2.4.3-scratch-r2` |
| `kubeops.image.pullPolicy` | Kubeops image pull policy | `IfNotPresent` |
| `kubeops.image.pullSecrets` | Kubeops image pull secrets | `[]` |
| `kubeops.namespaceHeaderName` | Additional header name for trusted namespaces | `""` |
Expand Down Expand Up @@ -361,7 +361,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `assetsvc.enabled` | Specifies whether this deprecated component should be installed. | `false` |
| `assetsvc.image.registry` | Kubeapps Assetsvc image registry | `docker.io` |
| `assetsvc.image.repository` | Kubeapps Assetsvc image repository | `bitnami/kubeapps-assetsvc` |
| `assetsvc.image.tag` | Kubeapps Assetsvc image tag (immutable tags are recommended) | `2.4.3-scratch-r1` |
| `assetsvc.image.tag` | Kubeapps Assetsvc image tag (immutable tags are recommended) | `2.4.3-scratch-r2` |
| `assetsvc.image.pullPolicy` | Kubeapps Assetsvc image pull policy | `IfNotPresent` |
| `assetsvc.image.pullSecrets` | Kubeapps Assetsvc image pull secrets | `[]` |
| `assetsvc.replicaCount` | Number of Assetsvc replicas to deploy | `1` |
Expand Down Expand Up @@ -416,7 +416,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `authProxy.enabled` | Specifies whether Kubeapps should configure OAuth login/logout | `false` |
| `authProxy.image.registry` | OAuth2 Proxy image registry | `docker.io` |
| `authProxy.image.repository` | OAuth2 Proxy image repository | `bitnami/oauth2-proxy` |
| `authProxy.image.tag` | OAuth2 Proxy image tag (immutable tags are recommended) | `7.2.1-debian-10-r56` |
| `authProxy.image.tag` | OAuth2 Proxy image tag (immutable tags are recommended) | `7.2.1-debian-10-r62` |
| `authProxy.image.pullPolicy` | OAuth2 Proxy image pull policy | `IfNotPresent` |
| `authProxy.image.pullSecrets` | OAuth2 Proxy image pull secrets | `[]` |
| `authProxy.external` | Use an external Auth Proxy instead of deploying its own one | `false` |
Expand Down Expand Up @@ -449,7 +449,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `pinnipedProxy.enabled` | Specifies whether Kubeapps should configure Pinniped Proxy | `false` |
| `pinnipedProxy.image.registry` | Pinniped Proxy image registry | `docker.io` |
| `pinnipedProxy.image.repository` | Pinniped Proxy image repository | `bitnami/kubeapps-pinniped-proxy` |
| `pinnipedProxy.image.tag` | Pinniped Proxy image tag (immutable tags are recommended) | `2.4.3-debian-10-r4` |
| `pinnipedProxy.image.tag` | Pinniped Proxy image tag (immutable tags are recommended) | `2.4.3-debian-10-r11` |
| `pinnipedProxy.image.pullPolicy` | Pinniped Proxy image pull policy | `IfNotPresent` |
| `pinnipedProxy.image.pullSecrets` | Pinniped Proxy image pull secrets | `[]` |
| `pinnipedProxy.defaultPinnipedNamespace` | Specify the (default) namespace in which pinniped concierge is installed | `pinniped-concierge` |
Expand All @@ -476,7 +476,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `rbac.create` | Specifies whether RBAC resources should be created | `true` |
| `testImage.registry` | NGINX image registry | `docker.io` |
| `testImage.repository` | NGINX image repository | `bitnami/nginx` |
| `testImage.tag` | NGINX image tag (immutable tags are recommended) | `1.21.6-debian-10-r21` |
| `testImage.tag` | NGINX image tag (immutable tags are recommended) | `1.21.6-debian-10-r27` |
| `testImage.pullPolicy` | NGINX image pull policy | `IfNotPresent` |
| `testImage.pullSecrets` | NGINX image pull secrets | `[]` |

Expand All @@ -501,7 +501,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith

| Name | Description | Value |
| ----------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | ----------------------- |
| `kubeappsapis.enabledPlugins` | Manually override which plugins are enabled for the Kubeapps-APIs service | `nil` |
| `kubeappsapis.enabledPlugins` | Manually override which plugins are enabled for the Kubeapps-APIs service | `[]` |
| `kubeappsapis.pluginConfig.core.packages.v1alpha1.versionsInSummary.major` | Number of major versions to display in the summary | `3` |
| `kubeappsapis.pluginConfig.core.packages.v1alpha1.versionsInSummary.minor` | Number of minor versions to display in the summary | `3` |
| `kubeappsapis.pluginConfig.core.packages.v1alpha1.versionsInSummary.patch` | Number of patch versions to display in the summary | `3` |
Expand All @@ -511,7 +511,7 @@ Once you have installed Kubeapps follow the [Getting Started Guide](https://gith
| `kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultAllowDowngrades` | Default policy for allowing applications to be downgraded to previous versions | `false` |
| `kubeappsapis.image.registry` | Kubeapps-APIs image registry | `docker.io` |
| `kubeappsapis.image.repository` | Kubeapps-APIs image repository | `bitnami/kubeapps-apis` |
| `kubeappsapis.image.tag` | Kubeapps-APIs image tag (immutable tags are recommended) | `2.4.3-debian-10-r7` |
| `kubeappsapis.image.tag` | Kubeapps-APIs image tag (immutable tags are recommended) | `2.4.3-debian-10-r14` |
| `kubeappsapis.image.pullPolicy` | Kubeapps-APIs image pull policy | `IfNotPresent` |
| `kubeappsapis.image.pullSecrets` | Kubeapps-APIs image pull secrets | `[]` |
| `kubeappsapis.replicaCount` | Number of frontend replicas to deploy | `2` |
Expand Down Expand Up @@ -1015,4 +1015,4 @@ Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
limitations under the License.
4 changes: 2 additions & 2 deletions bitnami/kubeapps/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ kubeappsapis:
## set this value if you want to manually override the list of plugins
## enabled for the service.
##
enabledPlugins:
enabledPlugins: []
pluginConfig:
core:
packages:
Expand All @@ -1656,7 +1656,7 @@ kubeappsapis:
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultUpgradePolicy Default upgrade policy generating version constraints
## enum: [ "major", "minor", "patch", "none" ]
defaultUpgradePolicy: none
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection Default policy for allowing prereleases containing one of the identifiers
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection [nullable] Default policy for allowing prereleases containing one of the identifiers
Copy link
Contributor

@miguelaeh miguelaeh Feb 25, 2022

Choose a reason for hiding this comment

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

If this field is a string, you would need to add:

Suggested change
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection [nullable] Default policy for allowing prereleases containing one of the identifiers
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection [string,nullable] Default policy for allowing prereleases containing one of the identifiers

in order to show type string into the schema

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This param can be:

defaultPrereleasesVersionSelection: null
defaultPrereleasesVersionSelection: []
defaultPrereleasesVersionSelection: ["foo", "bar"]

So, I'm not sure we would want to show type: string in the schema, perhaps

Suggested change
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection [nullable] Default policy for allowing prereleases containing one of the identifiers
## @param kubeappsapis.pluginConfig.kappController.packages.v1alpha1.defaultPrereleasesVersionSelection [array, nullable] Default policy for allowing prereleases containing one of the identifiers

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, then you should add array where I put string. You assumption is correct, just do not add spaces (I am not sure right now if there is a trim into the code for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added it, but it results in an invalid openapi schema: the required items property for arrays is not being generated.

With @param .... [nullable], valid schema, but not representing the reality

"defaultPrereleasesVersionSelection": {
    "type": "object", 
    "description": "Default policy for allowing prereleases containing one of the identifiers",
    "default": "null",
    "nullable": true
},

With @param .... [array, nullable], invalid schema

"defaultPrereleasesVersionSelection": {
    "type": "array", 
     // missing 'items' property
    "description": "Default policy for allowing prereleases containing one of the identifiers",
    "default": "null",
    "nullable": true
},

Expected valid OpenAPI 3.0.X schema:

Note that we are explicitly not targeting openapi 3.1, otherwise, the nullable prop would be removed, and the type object would become an array. Useful SO post clarifying the differences.

"defaultPrereleasesVersionSelection": {
    "type": "string",
    "items": {
      "type": "string" // how can we infer this value? 
    },
    "description": "Default policy for allowing prereleases containing one of the identifiers",
    "default": null,
    "nullable": true
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, rather than generating an invalid schema, I'd go with leaving it as is, until the items can be added/inferred in this edge case. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like another corner case for arrays. Thank you for sharing it.

So, rather than generating an invalid schema, I'd go with leaving it as is,

Currently, if you don't specify any modifier for setting the type, it will be object by default. I think if you add just [nullable] the type will be set to object into the schema. Is that what you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as we are not using the schema currently, semantic correctness is not a priority, so having type: object is something bearable until this edge case gets sorted out.
I mean, sacrificing semantic over syntactic correctness is preferred in this case.

Do you want me to file an issue in the readme's repo? Can we merge this PR then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, this corner case is going to be resolved soon in the readme-generator tool, therefore, let me update the PR to also include the [array] modifier.
Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, let's proceed with this PR, I will try to fix t

## ref: https://carvel.dev/kapp-controller/docs/latest/package-consumer-concepts/#prereleases
## e.g:
# defaultPrereleasesVersionSelection:
Expand Down