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

feat: Implement the GetHelmRelease endpoint #1534

Merged
merged 4 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions api/core/core.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ service Core {
get : "/v1/kustomizations/{name}"
};
}

/*
* ListHelmReleases lists helm releases from a cluster.
*/
Expand All @@ -48,6 +49,17 @@ service Core {
};
}


/*
* GetHelmRelease gets data about a single HelmRelease from the cluster.
*/
rpc GetHelmRelease(GetHelmReleaseRequest) returns (GetHelmReleaseResponse) {
option (google.api.http) = {
get : "/v1/helmrelease/{name}"
};
}


// Sources

/*
Expand Down Expand Up @@ -143,6 +155,15 @@ message ListHelmReleasesResponse {
repeated HelmRelease helm_releases = 1;
}

message GetHelmReleaseRequest {
string name = 1;
string namespace = 2;
}

message GetHelmReleaseResponse {
HelmRelease helm_release = 1;
}

message ListGitRepositoriesRequest {
string namespace = 1;
}
Expand Down
45 changes: 45 additions & 0 deletions api/core/core.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,43 @@
]
}
},
"/v1/helmrelease/{name}": {
"get": {
"summary": "GetHelmRelease gets data about a single HelmRelease from the cluster.",
"operationId": "Core_GetHelmRelease",
"responses": {
"200": {
"description": "A successful response.",
"schema": {
"$ref": "#/definitions/v1GetHelmReleaseResponse"
}
},
"default": {
"description": "An unexpected error response.",
"schema": {
"$ref": "#/definitions/rpcStatus"
}
}
},
"parameters": [
{
"name": "name",
"in": "path",
"required": true,
"type": "string"
},
{
"name": "namespace",
"in": "query",
"required": false,
"type": "string"
}
],
"tags": [
"Core"
]
}
},
"/v1/helmreleases": {
"get": {
"summary": "ListHelmReleases lists helm releases from a cluster.",
Expand Down Expand Up @@ -545,6 +582,14 @@
}
}
},
"v1GetHelmReleaseResponse": {
"type": "object",
"properties": {
"helmRelease": {
"$ref": "#/definitions/v1HelmRelease"
}
}
},
"v1GetKustomizationResponse": {
"type": "object",
"properties": {
Expand Down
148 changes: 148 additions & 0 deletions core/server/helm_release.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package server

import (
"bytes"
"compress/gzip"
"context"
"encoding/base64"
"encoding/json"
"fmt"
"io"
"strings"

"github.com/fluxcd/helm-controller/api/v2beta1"
helmv2 "github.com/fluxcd/helm-controller/api/v2beta1"
"github.com/fluxcd/pkg/ssa"
"github.com/weaveworks/weave-gitops/core/server/types"
pb "github.com/weaveworks/weave-gitops/pkg/api/core"
v1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func (cs *coreServer) ListHelmReleases(ctx context.Context, msg *pb.ListHelmReleasesRequest) (*pb.ListHelmReleasesResponse, error) {
k8s, err := cs.k8s.Client(ctx)
if err != nil {
return nil, doClientError(err)
}

l := &helmv2.HelmReleaseList{}

if err := list(ctx, k8s, temporarilyEmptyAppName, msg.Namespace, l); err != nil {
return nil, err
}

var results []*pb.HelmRelease
for _, helmRelease := range l.Items {
results = append(results, types.HelmReleaseToProto(&helmRelease, []*pb.GroupVersionKind{}))
}

return &pb.ListHelmReleasesResponse{
HelmReleases: results,
}, nil
}

func (cs *coreServer) GetHelmRelease(ctx context.Context, msg *pb.GetHelmReleaseRequest) (*pb.GetHelmReleaseResponse, error) {
k8s, err := cs.k8s.Client(ctx)
if err != nil {
return nil, doClientError(err)
}

helmRelease := helmv2.HelmRelease{}

if err = get(ctx, k8s, msg.Name, msg.Namespace, &helmRelease); err != nil {
return nil, err
}

inventory, err := getHelmReleaseInventory(ctx, helmRelease, k8s)
if err != nil {
return nil, err
}

return &pb.GetHelmReleaseResponse{
HelmRelease: types.HelmReleaseToProto(&helmRelease, inventory),
}, err
}

func getHelmReleaseInventory(ctx context.Context, helmRelease v2beta1.HelmRelease, k8s client.Client) ([]*pb.GroupVersionKind, error) {
storageNamespace := helmRelease.GetNamespace()
if helmRelease.Spec.StorageNamespace != "" {
storageNamespace = helmRelease.Spec.StorageNamespace
}

storageName := helmRelease.GetName()
if helmRelease.Spec.ReleaseName != "" {
storageName = helmRelease.Spec.ReleaseName
}

storageVersion := helmRelease.Status.LastReleaseRevision
if storageVersion < 1 {
// skip release if it failed to install
return nil, nil
}

storageSecret := v1.Secret{}
secretName := fmt.Sprintf("sh.helm.release.v1.%s.v%v", storageName, storageVersion)

if err := get(ctx, k8s, secretName, storageNamespace, &storageSecret); err != nil {
return nil, err
}

releaseData, releaseFound := storageSecret.Data["release"]
if !releaseFound {
return nil, fmt.Errorf("failed to decode the Helm storage object for HelmRelease '%s'", helmRelease.Name)
}

byteData, err := base64.StdEncoding.DecodeString(string(releaseData))
if err != nil {
return nil, err
}

var magicGzip = []byte{0x1f, 0x8b, 0x08}
if bytes.Equal(byteData[0:3], magicGzip) {
r, err := gzip.NewReader(bytes.NewReader(byteData))
if err != nil {
return nil, err
}

defer r.Close()

uncompressedByteData, err := io.ReadAll(r)
if err != nil {
return nil, err
}

byteData = uncompressedByteData
}

storage := types.HelmReleaseStorage{}
if err := json.Unmarshal(byteData, &storage); err != nil {
return nil, fmt.Errorf("failed to decode the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err)
}

objects, err := ssa.ReadObjects(strings.NewReader(storage.Manifest))
if err != nil {
return nil, fmt.Errorf("failed to read the Helm storage object for HelmRelease '%s': %w", helmRelease.Name, err)
}

var gvk []*pb.GroupVersionKind

found := map[string]bool{}
Copy link
Contributor

Choose a reason for hiding this comment

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

we could probs use map[string]struct{} here if we care about speed and memory, but bool reads better so happy either way.


for _, entry := range objects {
entry.GetAPIVersion()

idstr := strings.Join([]string{entry.GetAPIVersion(), entry.GetKind()}, "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why we call entry.GetAPIVersion here and then again on 134? I can see we are doing it somewhere else in the code, so am curious what repeating but not keeping the value gives us.

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 have no idea, and because I have no idea, I just used the same logic XD

Copy link
Contributor

Choose a reason for hiding this comment

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

does it hurt anything if you remove the first one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

while testing it does not hurt, but I don't know if it does with real k8s objects coming from a real k8s cluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

ack, makes sense, would be good to prove and remove at somepoint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed... Why?

entry.GetAPIVersion()

idstr := strings.Join([]string{entry.GetAPIVersion(), entry.GetKind()}, "_")

The function call sequence is:

  1. entry.GetAPIVersion()
  2. entry.GetKind()
  3. strings.Join()

With that extra call the difference is an extra call only:

  1. entry.GetAPIVersion()
  2. entry.GetAPIVersion()
  3. entry.GetKind()
  4. strings.Join()

I see only one case when it can have any effects, if the underlying code is far from optimal and has an internal counter (how many times it was called) or something, and it does not make sense.


if !found[idstr] {
found[idstr] = true

gvk = append(gvk, &pb.GroupVersionKind{
Group: entry.GroupVersionKind().Group,
Version: entry.GroupVersionKind().Version,
Kind: entry.GroupVersionKind().Kind,
})
}
}

return gvk, nil
}
109 changes: 109 additions & 0 deletions core/server/helm_release_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package server
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this server_test? we are testing an exposed interface, so we can test against it from the outside

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'm 100% on the that side (blackbox testing), but the rest of the code follows this logic (whitebox testing), so I did the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeh. i would be good to move over while we are starting new so we don;t blur lines

Copy link
Contributor Author

@yitsushi yitsushi Feb 28, 2022

Choose a reason for hiding this comment

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

Did a few pokes on it, all the tests are using the same functions (and global variable 😞 ), so I moved everything and now it's all server_test


import (
"context"
"testing"

helmv2 "github.com/fluxcd/helm-controller/api/v2beta1"
. "github.com/onsi/gomega"
pb "github.com/weaveworks/weave-gitops/pkg/api/core"
"github.com/weaveworks/weave-gitops/pkg/kube"
"k8s.io/apimachinery/pkg/util/rand"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func TestListHelmReleases(t *testing.T) {
g := NewGomegaWithT(t)
ctx := context.Background()

c, cleanup := makeGRPCServer(k8sEnv.Rest, t)
defer cleanup()

_, k, err := kube.NewKubeHTTPClientWithConfig(k8sEnv.Rest, "")
g.Expect(err).NotTo(HaveOccurred())

appName := "myapp"
ns := newNamespace(ctx, k, g)

newHelmRelease(ctx, appName, ns.Name, k, g)

res, err := c.ListHelmReleases(ctx, &pb.ListHelmReleasesRequest{
Namespace: ns.Name,
})
g.Expect(err).NotTo(HaveOccurred())
g.Expect(res.HelmReleases).To(HaveLen(1))
g.Expect(res.HelmReleases[0].Name).To(Equal(appName))
}

func TestGetHelmRelease(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

are you going to test any of the getHelmReleaseInventory bits? it feels important to us that we are populating that bit of the returned value correct

Copy link
Contributor

Choose a reason for hiding this comment

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

ah just literally saw your comment on that 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@yitsushi Here is how I had to test statuses:

g.Expect(k.Status().Patch(ctx, kust, client.Apply, opt...)).To(Succeed())

Note that this is the second .Patch, after creating the record initially with the first .Patch (instead of .Create).

Copy link
Contributor Author

@yitsushi yitsushi Feb 28, 2022

Choose a reason for hiding this comment

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

My issue with testing that function... well I have to create a full secret + store + data + "i don't know what else", because unlike kustomization, to get helm inventory, we query the api to get a helm secret, get release from the secret storage, decode + unzip (if gzipped), and finally read object from the storage. or something like that... so that would require a huge bootstrapping. I'll try it, because I would like to test it.

g := NewGomegaWithT(t)
ctx := context.Background()

c, cleanup := makeGRPCServer(k8sEnv.Rest, t)
defer cleanup()

_, k, err := kube.NewKubeHTTPClientWithConfig(k8sEnv.Rest, "")
g.Expect(err).NotTo(HaveOccurred())

appName := "myapp" + rand.String(5)
ns1 := newNamespace(ctx, k, g)
ns2 := newNamespace(ctx, k, g)
ns3 := newNamespace(ctx, k, g)

newHelmRelease(ctx, appName, ns1.Name, k, g)
newHelmRelease(ctx, appName, ns2.Name, k, g)

// Get app from ns1.
response, err := c.GetHelmRelease(ctx, &pb.GetHelmReleaseRequest{
Name: appName,
Namespace: ns1.Name,
})

g.Expect(err).NotTo(HaveOccurred())
g.Expect(response.HelmRelease.Name).To(Equal(appName))
g.Expect(response.HelmRelease.Namespace).To(Equal(ns1.Name))

// Get app from ns2.
response, err = c.GetHelmRelease(ctx, &pb.GetHelmReleaseRequest{
Name: appName,
Namespace: ns2.Name,
})

g.Expect(err).NotTo(HaveOccurred())
g.Expect(response.HelmRelease.Name).To(Equal(appName))
g.Expect(response.HelmRelease.Namespace).To(Equal(ns2.Name))

// Get app from ns3, should fail.
_, err = c.GetHelmRelease(ctx, &pb.GetHelmReleaseRequest{
Name: appName,
Namespace: ns3.Name,
})

g.Expect(err).To(HaveOccurred())
}

func newHelmRelease(
ctx context.Context,
appName, nsName string,
k client.Client,
g *GomegaWithT,
) helmv2.HelmRelease {
release := helmv2.HelmRelease{
Spec: helmv2.HelmReleaseSpec{
Chart: helmv2.HelmChartTemplate{
Spec: helmv2.HelmChartTemplateSpec{
SourceRef: helmv2.CrossNamespaceObjectReference{
Kind: "GitRepository",
Name: "somesource",
},
},
},
},
}
release.Name = appName
release.Namespace = nsName

g.Expect(k.Create(ctx, &release)).To(Succeed())

return release
}
23 changes: 0 additions & 23 deletions core/server/automations.go → core/server/kustomization.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"

helmv2 "github.com/fluxcd/helm-controller/api/v2beta1"
kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta2"
"github.com/weaveworks/weave-gitops/core/server/types"
pb "github.com/weaveworks/weave-gitops/pkg/api/core"
Expand Down Expand Up @@ -57,25 +56,3 @@ func (cs *coreServer) GetKustomization(ctx context.Context, msg *pb.GetKustomiza

return &pb.GetKustomizationResponse{Kustomization: res}, nil
}

func (cs *coreServer) ListHelmReleases(ctx context.Context, msg *pb.ListHelmReleasesRequest) (*pb.ListHelmReleasesResponse, error) {
k8s, err := cs.k8s.Client(ctx)
if err != nil {
return nil, doClientError(err)
}

l := &helmv2.HelmReleaseList{}

if err := list(ctx, k8s, temporarilyEmptyAppName, msg.Namespace, l); err != nil {
return nil, err
}

var results []*pb.HelmRelease
for _, repository := range l.Items {
results = append(results, types.HelmReleaseToProto(&repository))
}

return &pb.ListHelmReleasesResponse{
HelmReleases: results,
}, nil
}