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

admin/history: fast-follow PR feedback #9856

Merged
merged 4 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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
Loading