Skip to content

Commit

Permalink
Env var escape hatches (#526)
Browse files Browse the repository at this point in the history
* Add env var to escape new default behavior

* Add changelog
  • Loading branch information
Kevin Dorosh authored Sep 29, 2022
1 parent 329e29b commit 3755d8b
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 4 deletions.
5 changes: 5 additions & 0 deletions changelog/v0.30.7/escape-hatches.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
changelog:
- type: FIX
resolvesIssue: false
description: Add escape hatches via env var for any users depending on large statuses
issueLink: https://github.com/solo-io/solo-kit/issues/523
14 changes: 12 additions & 2 deletions pkg/api/shared/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"os"

"github.com/golang/protobuf/jsonpb"
"github.com/solo-io/go-utils/contextutils"
Expand All @@ -20,6 +21,15 @@ const (
MaxStatusBytes = 2048
)

var (
// only public for unit tests!
DisableMaxStatusSize = false
)

func init() {
DisableMaxStatusSize = os.Getenv("DISABLE_MAX_STATUS_SIZE") == "true"
}

// ApplyStatus is used by clients that don't support patch updates to resource statuses (e.g. consul, files, in-memory)
func ApplyStatus(rc clients.ResourceClient, statusClient resources.StatusClient, inputResource resources.InputResource, opts clients.ApplyStatusOpts) (resources.Resource, error) {
name := inputResource.GetMetadata().GetName()
Expand Down Expand Up @@ -78,9 +88,9 @@ func GetJsonPatchData(ctx context.Context, inputResource resources.InputResource
patch := fmt.Sprintf(`[{"op": "replace", "path": "/status/statuses/%s", "value": %s}]`, ns, string(bytes)) // only replace our status so other reporters are not affected (e.g. blue-green of gloo)
data := []byte(patch)

if len(data) > MaxStatusBytes {
if !DisableMaxStatusSize && len(data) > MaxStatusBytes {
if contextutils.GetLogLevel() == zapcore.DebugLevel {
contextutils.LoggerFrom(ctx).Warnf("status patch is too large, will not apply: %s", data)
contextutils.LoggerFrom(ctx).Debugf("status patch is too large, will not apply: %s", data)
} else {
contextutils.LoggerFrom(ctx).Warnw("status patch is too large, will not apply", zap.Int("status_bytes", len(data)))
}
Expand Down
21 changes: 20 additions & 1 deletion pkg/api/v1/clients/kube/resource_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,6 @@ var _ = Describe("Test Kube ResourceClient", func() {
})

It("skips status updates if too large", func() {

var sb strings.Builder
for i := 0; i < shared.MaxStatusBytes+1; i++ {
sb.WriteString("a")
Expand All @@ -634,6 +633,26 @@ var _ = Describe("Test Kube ResourceClient", func() {
Expect(err).To(MatchError(ContainSubstring("patch is too large")))
Expect(res).To(BeNil())
})

It("honors env var override on status truncation", func() {

shared.DisableMaxStatusSize = true

var sb strings.Builder
for i := 0; i < shared.MaxStatusBytes+1; i++ {
sb.WriteString("a")
}
tooLargeReason := sb.String()

statusClient.SetStatus(resourceToUpdate, &core.Status{
State: 2,
Reason: tooLargeReason,
ReportedBy: "me",
})

_, err := rc.ApplyStatus(statusClient, resourceToUpdate, clients.ApplyStatusOpts{})
Expect(err).NotTo(MatchError(ContainSubstring("patch is too large")))
})
})

Describe("listing resources", func() {
Expand Down
14 changes: 13 additions & 1 deletion pkg/api/v2/reporter/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package reporter

import (
"context"
"os"
"sort"
"strings"

Expand All @@ -22,6 +23,15 @@ const (
MaxStatusKeys = 100
)

var (
// only public for unit tests!
DisableTruncateStatus = false
)

func init() {
DisableTruncateStatus = os.Getenv("DISABLE_TRUNCATE_STATUS") == "true"
}

type Report struct {
Warnings []string
Errors error
Expand Down Expand Up @@ -268,7 +278,9 @@ func (r *reporter) WriteReports(ctx context.Context, resourceErrs ResourceReport
return errors.Errorf("reporter: was passed resource of kind %v but no client to support it", kind)
}
status := r.StatusFromReport(report, subresourceStatuses)
status = trimStatus(status)
if !DisableTruncateStatus {
status = trimStatus(status)
}
resourceStatus := r.statusClient.GetStatus(resource)

if status.Equal(resourceStatus) {
Expand Down
60 changes: 60 additions & 0 deletions pkg/api/v2/reporter/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,66 @@ var _ = Describe("Reporter", func() {
}))
})

It("honors override for truncate statuses", func() {
rep.DisableTruncateStatus = true

r1, err := mockResourceClient.Write(v1.NewMockResource("", "mocky"), clients.WriteOpts{})
Expect(err).NotTo(HaveOccurred())

var sb strings.Builder
for i := 0; i < rep.MaxStatusBytes+1; i++ {
sb.WriteString("a")
}

// an error larger than our max (1kb) that should NOT be truncated
veryLargeError := sb.String()

childSubresourceStatuses := map[string]*core.Status{}
for i := 0; i < rep.MaxStatusKeys+1; i++ { // we have numerous keys, and do not expect to trim to num(parentkeys)/2 (i.e. rep.MaxStatusKeys/2)
var sb strings.Builder
for j := 0; j <= i; j++ {
sb.WriteString("a")
}
childSubresourceStatuses[fmt.Sprintf("child-subresource-%s", sb.String())] = &core.Status{
State: core.Status_Warning,
Reason: veryLargeError,
ReportedBy: "test",
SubresourceStatuses: nil, // intentionally nil; only test recursive call once
}
}

subresourceStatuses := map[string]*core.Status{}
for i := 0; i < rep.MaxStatusKeys+1; i++ { // we have numerous keys, and do not expect to trim to 100 keys (rep.MaxStatusKeys)
var sb strings.Builder
for j := 0; j <= i; j++ {
sb.WriteString("a")
}
subresourceStatuses[fmt.Sprintf("parent-subresource-%s", sb.String())] = &core.Status{
State: core.Status_Warning,
Reason: veryLargeError,
ReportedBy: "test",
SubresourceStatuses: childSubresourceStatuses,
}
}

resourceErrs := rep.ResourceReports{
r1.(*v1.MockResource): rep.Report{Errors: fmt.Errorf(veryLargeError)},
}
err = reporter.WriteReports(context.TODO(), resourceErrs, subresourceStatuses)
Expect(err).NotTo(HaveOccurred())

r1, err = mockResourceClient.Read(r1.GetMetadata().Namespace, r1.GetMetadata().Name, clients.ReadOpts{})
Expect(err).NotTo(HaveOccurred())

status := statusClient.GetStatus(r1.(*v1.MockResource))
Expect(status).To(Equal(&core.Status{
State: 2,
Reason: veryLargeError,
ReportedBy: "test",
SubresourceStatuses: subresourceStatuses,
}))
})

It("handles conflict", func() {
r1, err := mockResourceClient.Write(v1.NewMockResource("", "mocky"), clients.WriteOpts{})
Expect(err).NotTo(HaveOccurred())
Expand Down

0 comments on commit 3755d8b

Please sign in to comment.