-
Notifications
You must be signed in to change notification settings - Fork 702
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
Add GetPackageRepositories implementation for carvel and helm-fluxv2 #2840
Conversation
@@ -18,7 +18,7 @@ import ( | |||
"google.golang.org/grpc" | |||
|
|||
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime" | |||
v1alpha1 "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/plugins/helm_operator/packages/v1alpha1" | |||
v1alpha1 "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/plugins/helm_fluxv2/packages/v1alpha1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably just go with fluxv2, i.e. drop "helm". Based on their docs Flux supports more than just helm, they also support Git repositories with artifacts, as well as Amazon-style buckets with artifacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I was thinking that we're only interested in the helm support, but we may want to support any repositories/packages. Note that we'd need to then include something like a resource type field (group/version/name) so if we're returning results including both HelmRepositories and GitRepositories (?), they can be recognised as such (I'll just rename in this PR). Let me do the rename...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. :)
cmd/kubeapps-apis/README.md
Outdated
IMAGE_TAG=dev1 make kubeapps/kubeapps-apis | ||
``` | ||
|
||
and make that imageg available on your cluster somehow. If using kind, you can simply do: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling and/or wording?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I viewed and I am ok with the changes. I don't have enough expertise on grpc and protobuf to be able to comment constructively at this point, but I will get there someday :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry missed one question. In earlier version of the PR, I saw a change you made in deploy-dev.mk to deploy kapp-controller to a cluster. I don't see that anymore in this list of files, nor do I see a target to deploy flux v2 Is that deliberate?
…roller and helm_fluxv2 plugins.
4edf1f8
to
bf9a383
Compare
That was in the previous PR, let me add a similar one with the flux command I used to install the flux CRDs etc., as it's also useful as documentation of what version etc. was being used. |
Description of the change
This was just to rename the
helm_operator
plugin tohelm_fluxv2
, but while there, I've added a demo implementation for theGetPackageRepositories
method for both.From the updated README, with the appropriate resources on your cluster you can now:
Benefits
Provides an example of an API call implemented by two plugins (needed for #2775). Renames helm flux plugin more appropriately (@gfichtenholt found helm_operator is from a pre fluxv2 version). Provides an example so @gfichtenholt can think about GetAvailablePackages for helm-fluxv2 (though as I mentioned on slack, this may be more work than we'd thought, depending whether fluxv2 actually exposes something analogous - afaict so far, it only creates a
HelmChart
object when a user installs from a repo, happy to jump in there to help as needed).Possible drawbacks
N/A
Applicable issues
Additional information
Next up for me:
/core/packages/v1alpha1/GetPackageRepositories
to show how we could return objects across plugins (chatted with @migmartri about this).But I'll discuss the above as part of our iteration planning next week.