[Feature] [Platform] Install multi type support #1992
Conversation
ajanikow
force-pushed
the
feature/platform/extend_install_commands
branch
from
05f043b to
e6f0741
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for multiple installation types for platform packages, allowing packages to be sourced from OCI registries, Helm index repositories, remote URLs, local files, or inline base64-encoded data. Previously, packages could only be sourced from the Platform LicenseManager or provided as raw data.
- Added
PackageTypeenum andResolvePackageSpecfunction to handle different package sources - Changed
PackageSpec.Chartfield fromsharedApi.Datato*stringto support URL/path specifications - Enhanced CLI with identity logging and improved flag validation
- Updated documentation with examples for all supported package types
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/util/k8sutil/helm/package.go | Added PackageType enum, validation logic, and changed Chart field to support multiple modes |
| pkg/platform/pack/utils.go | Implemented ResolvePackageSpec to handle different package types (OCI, file, remote, index, inline, platform) |
| pkg/util/cli/registry.go | Removed debug logging configuration |
| pkg/util/cli/lm.go | Added identity logging for license manager client |
| pkg/util/cli/flag.go | Refactored validation to use new Get method and renamed internal get method |
| pkg/util/errors/errors.go | Added debug println statement (appears to be debugging code) |
| pkg/platform/package_test.go | Updated test data to use base64 encoding for inline charts |
| pkg/platform/package_install.go | Simplified to use ResolvePackageSpec |
| pkg/platform/pack/import.go | Updated to use base64 encoding and added blob existence check |
| pkg/platform/pack/export.go | Refactored to use ResolvePackageSpec |
| pkg/license_manager/client.go | Added Identity method to client interface |
| internal/docs_test.go | Refactored documentation generation and added secondary API docs test |
| docs/platform.install.md | New documentation file with package installation schema |
| docs/cli/arangodb_operator_platform.md | Updated CLI documentation with default values |
| go.mod, go.sum | Updated dependencies |
| .gitignore | Added cache directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ajanikow
force-pushed
the
feature/platform/extend_install_commands
branch
from
e6f0741 to
fe57de6
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return io.ReadAll(resp.Body) | ||
|
|
There was a problem hiding this comment.
Missing HTTP response status check. The code should verify that resp.StatusCode is in the 2xx range before attempting to read the body. Without this check, error responses (4xx, 5xx) will be treated as valid chart data.
| return io.ReadAll(resp.Body) | |
| if resp.StatusCode < 200 || resp.StatusCode >= 300 { | |
| body, _ := io.ReadAll(resp.Body) | |
| return nil, fmt.Errorf("failed to fetch remote chart: status %d: %s", resp.StatusCode, string(body)) | |
| } | |
| return io.ReadAll(resp.Body) |
| // - If starts with `file://` chart is fetched from local FileSystem | ||
| // - If starts with `http://` or `https://` chart is fetched from the remote URL | ||
| // - If starts with `index://` chart is fetched using Helm YAML Index File structure (using version and name) | ||
| // - If Starts with `oci://` chart is fetched from Registry Compatible OCI Repository |
There was a problem hiding this comment.
Inconsistent capitalization: "If Starts with oci://" should be "If starts with oci://" (lowercase 's' to match the pattern used in lines 82-84).
| // - If Starts with `oci://` chart is fetched from Registry Compatible OCI Repository | |
| // - If starts with `oci://` chart is fetched from Registry Compatible OCI Repository |
| - If starts with `file://` chart is fetched from local FileSystem | ||
| - If starts with `http://` or `https://` chart is fetched from the remote URL | ||
| - If starts with `index://` chart is fetched using Helm YAML Index File structure (using version and name) | ||
| - If Starts with `oci://` chart is fetched from Registry Compatible OCI Repository |
There was a problem hiding this comment.
The same spelling inconsistency appears in the documentation: "If Starts with oci://" should be "If starts with oci://" (lowercase 's' to match lines 42-43).
| - If Starts with `oci://` chart is fetched from Registry Compatible OCI Repository | |
| - If starts with `oci://` chart is fetched from Registry Compatible OCI Repository |
No description provided.