Skip to content

Commit 3395408

Browse files
committed
feature: add warning handler
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
1 parent d19ca44 commit 3395408

File tree

4 files changed

+235
-24
lines changed

4 files changed

+235
-24
lines changed

cmd/oras/internal/option/binary_target.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package option
1818
import (
1919
"errors"
2020
"fmt"
21-
"sync"
2221

2322
"github.com/spf13/cobra"
2423
"github.com/spf13/pflag"
@@ -60,8 +59,6 @@ func (target *BinaryTarget) ApplyFlags(fs *pflag.FlagSet) {
6059

6160
// Parse parses user-provided flags and arguments into option struct.
6261
func (target *BinaryTarget) Parse(cmd *cobra.Command) error {
63-
target.From.warned = make(map[string]*sync.Map)
64-
target.To.warned = target.From.warned
6562
// resolve are parsed in array order, latter will overwrite former
6663
target.From.resolveFlag = append(target.resolveFlag, target.From.resolveFlag...)
6764
target.To.resolveFlag = append(target.resolveFlag, target.To.resolveFlag...)

cmd/oras/internal/option/remote.go

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"os"
2727
"strconv"
2828
"strings"
29-
"sync"
3029

3130
"github.com/sirupsen/logrus"
3231
"github.com/spf13/cobra"
@@ -38,6 +37,7 @@ import (
3837
"oras.land/oras-go/v2/registry/remote/errcode"
3938
"oras.land/oras-go/v2/registry/remote/retry"
4039
oerrors "oras.land/oras/cmd/oras/internal/errors"
40+
"oras.land/oras/cmd/oras/internal/warning"
4141
"oras.land/oras/internal/credential"
4242
"oras.land/oras/internal/crypto"
4343
onet "oras.land/oras/internal/net"
@@ -74,7 +74,6 @@ type Remote struct {
7474
applyDistributionSpec bool
7575
headerFlags []string
7676
headers http.Header
77-
warned map[string]*sync.Map
7877
plainHTTP func() (plainHTTP bool, enforced bool)
7978
store credentials.Store
8079
}
@@ -324,23 +323,6 @@ func (remo *Remote) Credential() auth.Credential {
324323
return credential.Credential(remo.Username, remo.Secret)
325324
}
326325

327-
func (remo *Remote) handleWarning(registry string, logger logrus.FieldLogger) func(warning remote.Warning) {
328-
if remo.warned == nil {
329-
remo.warned = make(map[string]*sync.Map)
330-
}
331-
warned := remo.warned[registry]
332-
if warned == nil {
333-
warned = &sync.Map{}
334-
remo.warned[registry] = warned
335-
}
336-
logger = logger.WithField("registry", registry)
337-
return func(warning remote.Warning) {
338-
if _, loaded := warned.LoadOrStore(warning.WarningValue, struct{}{}); !loaded {
339-
logger.Warn(warning.Text)
340-
}
341-
}
342-
}
343-
344326
// NewRegistry assembles a oras remote registry.
345327
func (remo *Remote) NewRegistry(registry string, common Common, logger logrus.FieldLogger) (reg *remote.Registry, err error) {
346328
reg, err = remote.NewRegistry(registry)
@@ -349,7 +331,7 @@ func (remo *Remote) NewRegistry(registry string, common Common, logger logrus.Fi
349331
}
350332
registry = reg.Reference.Registry
351333
reg.PlainHTTP = remo.isPlainHttp(registry)
352-
reg.HandleWarning = remo.handleWarning(registry, logger)
334+
reg.HandleWarning = warning.GetHandler(registry, logger)
353335
if reg.Client, err = remo.authClient(registry, common.Debug); err != nil {
354336
return nil, err
355337
}
@@ -367,7 +349,7 @@ func (remo *Remote) NewRepository(reference string, common Common, logger logrus
367349
}
368350
registry := repo.Reference.Registry
369351
repo.PlainHTTP = remo.isPlainHttp(registry)
370-
repo.HandleWarning = remo.handleWarning(registry, logger)
352+
repo.HandleWarning = warning.GetHandler(registry, logger)
371353
if repo.Client, err = remo.authClient(registry, common.Debug); err != nil {
372354
return nil, err
373355
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
Copyright The ORAS Authors.
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
*/
15+
16+
package warning
17+
18+
import (
19+
"sync"
20+
21+
"github.com/sirupsen/logrus"
22+
"oras.land/oras-go/v2/registry/remote"
23+
)
24+
25+
var (
26+
oneHandler warningHandler
27+
)
28+
29+
type warningHandler struct {
30+
warned sync.Map
31+
}
32+
33+
func GetHandler(registry string, logger logrus.FieldLogger) func(warning remote.Warning) {
34+
result, _ := oneHandler.warned.LoadOrStore(registry, &sync.Map{})
35+
warner := result.(*sync.Map)
36+
return func(warning remote.Warning) {
37+
if _, loaded := warner.LoadOrStore(warning.WarningValue, true); !loaded {
38+
logger.WithField("registry", registry).Warn(warning.Text)
39+
}
40+
}
41+
}
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
/*
2+
Copyright The ORAS Authors.
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
*/
15+
16+
package warning
17+
18+
import (
19+
"bytes"
20+
"strings"
21+
"sync"
22+
"testing"
23+
24+
"github.com/sirupsen/logrus"
25+
"oras.land/oras-go/v2/registry/remote"
26+
)
27+
28+
func TestWarningHandler_GetHandler(t *testing.T) {
29+
tests := []struct {
30+
name string
31+
registry string
32+
warnings []remote.Warning
33+
wantLogs []string
34+
wantCount int
35+
}{
36+
{
37+
name: "single warning",
38+
registry: "localhost:5000",
39+
warnings: []remote.Warning{
40+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "First warning"}},
41+
},
42+
wantLogs: []string{"First warning"},
43+
wantCount: 1,
44+
},
45+
{
46+
name: "duplicate warnings same registry",
47+
registry: "localhost:5000",
48+
warnings: []remote.Warning{
49+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "First warning"}},
50+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "First warning"}},
51+
},
52+
wantLogs: []string{"First warning"},
53+
wantCount: 1,
54+
},
55+
{
56+
name: "different warnings same registry",
57+
registry: "localhost:5000",
58+
warnings: []remote.Warning{
59+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "First warning"}},
60+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "Second warning"}},
61+
},
62+
wantLogs: []string{"First warning", "Second warning"},
63+
wantCount: 2,
64+
},
65+
{
66+
name: "empty warning value",
67+
registry: "localhost:5000",
68+
warnings: []remote.Warning{
69+
{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "Empty value warning"}},
70+
},
71+
wantLogs: []string{"Empty value warning"},
72+
wantCount: 1,
73+
},
74+
}
75+
76+
for _, tt := range tests {
77+
t.Run(tt.name, func(t *testing.T) {
78+
oneHandler = warningHandler{}
79+
var buf bytes.Buffer
80+
logger := logrus.New()
81+
logger.SetOutput(&buf)
82+
logger.SetLevel(logrus.WarnLevel)
83+
84+
handler := GetHandler(tt.registry, logger)
85+
86+
for _, warning := range tt.warnings {
87+
handler(warning)
88+
}
89+
90+
output := buf.String()
91+
logCount := strings.Count(output, "level=warning")
92+
93+
if logCount != tt.wantCount {
94+
t.Errorf("Expected %d warning logs, got %d", tt.wantCount, logCount)
95+
}
96+
97+
for _, expectedLog := range tt.wantLogs {
98+
if !strings.Contains(output, expectedLog) {
99+
t.Errorf("Expected log to contain %q, but it didn't. Output: %s", expectedLog, output)
100+
}
101+
}
102+
103+
if !strings.Contains(output, tt.registry) && tt.wantCount > 0 {
104+
t.Errorf("Expected log to contain registry %q, but it didn't. Output: %s", tt.registry, output)
105+
}
106+
})
107+
}
108+
}
109+
110+
func TestWarningHandler_GetHandler_DifferentRegistries(t *testing.T) {
111+
var buf bytes.Buffer
112+
logger := logrus.New()
113+
logger.SetOutput(&buf)
114+
logger.SetLevel(logrus.WarnLevel)
115+
116+
handler1 := GetHandler("registry1.example.com", logger)
117+
handler2 := GetHandler("registry2.example.com", logger)
118+
119+
warning := remote.Warning{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "Test warning"}}
120+
121+
handler1(warning)
122+
handler2(warning)
123+
124+
output := buf.String()
125+
logCount := strings.Count(output, "level=warning")
126+
127+
if logCount != 2 {
128+
t.Errorf("Expected 2 warning logs for different registries, got %d", logCount)
129+
}
130+
131+
if !strings.Contains(output, "registry1.example.com") {
132+
t.Error("Expected log to contain registry1.example.com")
133+
}
134+
if !strings.Contains(output, "registry2.example.com") {
135+
t.Error("Expected log to contain registry2.example.com")
136+
}
137+
}
138+
139+
func TestWarningHandler_GetHandler_Concurrency(t *testing.T) {
140+
var buf bytes.Buffer
141+
logger := logrus.New()
142+
logger.SetOutput(&buf)
143+
logger.SetLevel(logrus.WarnLevel)
144+
145+
handler := GetHandler("localhost:5000", logger)
146+
147+
var wg sync.WaitGroup
148+
numGoroutines := 100
149+
150+
for i := 0; i < numGoroutines; i++ {
151+
wg.Add(1)
152+
go func(i int) {
153+
defer wg.Done()
154+
warning := remote.Warning{
155+
WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "Concurrent warning"},
156+
}
157+
handler(warning)
158+
}(i)
159+
}
160+
161+
wg.Wait()
162+
163+
output := buf.String()
164+
logCount := strings.Count(output, "level=warning")
165+
166+
if logCount != 1 {
167+
t.Errorf("Expected exactly 1 warning log despite concurrent calls, got %d", logCount)
168+
}
169+
}
170+
171+
func TestWarningHandler_GetHandler_MultipleHandlersForSameRegistry(t *testing.T) {
172+
var buf bytes.Buffer
173+
logger := logrus.New()
174+
logger.SetOutput(&buf)
175+
logger.SetLevel(logrus.WarnLevel)
176+
177+
handler1 := GetHandler("localhost:5000", logger)
178+
handler2 := GetHandler("localhost:5000", logger)
179+
180+
warning := remote.Warning{WarningValue: remote.WarningValue{Code: 299, Agent: "oras", Text: "Test warning"}}
181+
182+
handler1(warning)
183+
handler2(warning)
184+
185+
output := buf.String()
186+
logCount := strings.Count(output, "level=warning")
187+
188+
if logCount != 1 {
189+
t.Errorf("Expected 1 warning log for same registry with multiple handlers, got %d", logCount)
190+
}
191+
}

0 commit comments

Comments
 (0)