Skip to content

Commit

Permalink
admin/history: fast-follow PR feedback (#9856)
Browse files Browse the repository at this point in the history
* fast follow to previous pr

* add changelog

* Update projects/gloo/pkg/servers/iosnapshot/resources.go

Co-authored-by: Jenny Shu <28537278+jenshu@users.noreply.github.com>

---------

Co-authored-by: Jenny Shu <28537278+jenshu@users.noreply.github.com>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Aug 2, 2024
1 parent f44e167 commit 77b72e6
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 17 deletions.
11 changes: 11 additions & 0 deletions changelog/v1.18.0-beta13/admin-server-p4.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
changelog:
- type: NON_USER_FACING
issueLink: https://github.com/solo-io/solo-projects/issues/6600
resolvesIssue: false
description: >-
Fast follow PR, to incorporate feedback from previous PR review.
- type: NON_USER_FACING
issueLink: https://github.com/solo-io/solo-projects/issues/6602
resolvesIssue: false
description: >-
Fast follow PR, to incorporate feedback from previous PR review.
4 changes: 2 additions & 2 deletions pkg/schemes/scheme.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
extauthkubev1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/extauth/v1/kube/apis/enterprise.gloo.solo.io/v1"
graphqlv1beta1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/enterprise/options/graphql/v1beta1/kube/apis/graphql.gloo.solo.io/v1beta1"
gloov1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1/kube/apis/gloo.solo.io/v1"
"github.com/solo-io/solo-apis/pkg/api/ratelimit.solo.io/v1alpha1"
ratelimitv1alpha1 "github.com/solo-io/solo-apis/pkg/api/ratelimit.solo.io/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -38,7 +38,7 @@ var SchemeBuilder = runtime.SchemeBuilder{
// These are packed in the OSS Helm Chart, and therefore we register the schemes here as well
graphqlv1beta1.AddToScheme,
extauthkubev1.AddToScheme,
v1alpha1.AddToScheme,
ratelimitv1alpha1.AddToScheme,
}

func AddToScheme(s *runtime.Scheme) error {
Expand Down
9 changes: 7 additions & 2 deletions pkg/utils/glooadminutils/admincli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@
This is the Go client that should be used whenever communicating with the Gloo Admin API. Within the Gloo project, it is used inside of tests.

### Philosophy
We expose methods that the client exposes which are [syntactic sugar](https://en.wikipedia.org/wiki/Syntactic_sugar) on top of the command API. These methods tend to follow the naming convention: `GetX`:
We expose methods that return a [Command](/pkg/utils/cmdutils/cmd.go) which can be run by the calling code. Any methods that fit this structure, should end in `Cmd`:
```go
func GetInputSnapshot(ctx context.Context) (string, error) {}
func InputSnapshotCmd(ctx context.Context) cmdutils.Cmd {}
```

There are also methods that the client exposes which are [syntactic sugar](https://en.wikipedia.org/wiki/Syntactic_sugar) on top of this command API. These methods tend to follow the naming convention: `GetX`:
```go
func GetInputSnapshot(ctx context.Context) ([]interface{}, error) {}
```
_As a general practice, these methods should return a concrete type, whenever possible._
13 changes: 2 additions & 11 deletions projects/gloo/pkg/servers/iosnapshot/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,14 @@ func (h *historyImpl) GetEdgeApiSnapshot(_ context.Context) SnapshotResponseData

// GetInputSnapshot gets the input snapshot for all components.
func (h *historyImpl) GetInputSnapshot(ctx context.Context) SnapshotResponseData {
inputSnapshotClient := h.getInputSnapshotClientSafe()
if inputSnapshotClient == nil {
if h.inputSnapshotClient == nil {
return errorSnapshotResponse(eris.New("No kubernetes Client found for InputSnapshot"))
}

var objects []client.Object
var errs *multierror.Error
for _, gvk := range h.inputSnapshotGvks {
gvkResources, err := h.listObjectsForGvk(ctx, inputSnapshotClient, gvk)
gvkResources, err := h.listObjectsForGvk(ctx, h.inputSnapshotClient, gvk)
if err != nil {
// We intentionally aggregate the errors so that we can return a "best effort" set of
// resources, and one error doesn't lead to the entire set of GVKs being short-circuited
Expand Down Expand Up @@ -274,14 +273,6 @@ func (h *historyImpl) listObjectsForGvk(ctx context.Context, cli client.Client,
return objects, errs.ErrorOrNil()
}

// getInputSnapshotClientSafe gets the Kubernetes client used for CRUD operations
// on resources used in the Kubernetes Gateway integration
func (h *historyImpl) getInputSnapshotClientSafe() client.Client {
h.RLock()
defer h.RUnlock()
return h.inputSnapshotClient
}

// redactApiSnapshot accepts an ApiSnapshot, and mutates it to remove sensitive data.
// It is critical that data which is exposed by this component is redacted,
// so that customers can feel comfortable sharing the results with us.
Expand Down
6 changes: 4 additions & 2 deletions projects/gloo/pkg/servers/iosnapshot/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ var (
gloov1.ProxyGVK,
}

// PolicyGVKs is the set of GVKs that are used by a classic Gloo Gateway installation,
// when only the Edge Gateway APIs are being used
// PolicyGVKs is the set of GVKs that are used by a classic Gloo Gateway installation.
// This is the common set of GVKs that are available when Edge Gateway APIs are being
// used. See KubernetesGatewayIntegrationPolicyGVKs for the set of GVKs that are added
// when the Kubernetes Gateway API is enabled
PolicyGVKs = []schema.GroupVersionKind{
gatewayv1.VirtualHostOptionGVK,
gatewayv1.RouteOptionGVK,
Expand Down

0 comments on commit 77b72e6

Please sign in to comment.