Add log entry when versions are suppressed #329
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package ingress | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "log" | ||
| "net/http" | ||
| ) | ||
|
|
||
| func (s *Server) handleMinPackageAge(w http.ResponseWriter, r *http.Request) { | ||
| var event MinPackageAgeEvent | ||
| if err := json.NewDecoder(r.Body).Decode(&event); err != nil { | ||
| http.Error(w, "invalid request body", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| log.Println("Got min-package-age event:", event) | ||
|
|
||
| stored := s.minAgeStore.Add(event) | ||
|
|
||
| // These events are informational only: they should show up in the Logs tab, | ||
| // but must not create a native popup notification. | ||
| go s.ui.NotifyMinPackageAge(stored) | ||
|
|
||
| w.WriteHeader(http.StatusOK) | ||
| } | ||
|
|
||
| func (s *Server) handleMinPackageAgeEvents(w http.ResponseWriter, r *http.Request) { | ||
| if !s.validateUIToken(w, r) { | ||
| return | ||
| } | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusOK) | ||
| if err := json.NewEncoder(w).Encode(s.minAgeStore.List()); err != nil { | ||
| log.Printf("failed to encode min-package-age events: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func (s *Server) handleGetMinPackageAgeEventByID(w http.ResponseWriter, r *http.Request) { | ||
| if !s.validateUIToken(w, r) { | ||
| return | ||
| } | ||
| id := r.PathValue("id") | ||
| if event, ok := s.minAgeStore.Get(id); ok { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| if err := json.NewEncoder(w).Encode(event); err != nil { | ||
| log.Printf("failed to encode min-package-age event %s: %v", id, err) | ||
| } | ||
| return | ||
| } | ||
| w.WriteHeader(http.StatusNotFound) | ||
| if _, err := w.Write([]byte("Event not found")); err != nil { | ||
| log.Printf("failed to write 404 response: %v", err) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package ingress | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "sync" | ||
| ) | ||
|
|
||
| type minPackageAgeEventStore struct { | ||
| mu sync.RWMutex | ||
| events []MinPackageAgeEvent | ||
| } | ||
|
|
||
| const minPackageAgeEventID = "min-package-age-suppressed" | ||
|
|
||
| func (e *minPackageAgeEventStore) Add(ev MinPackageAgeEvent) MinPackageAgeEvent { | ||
| e.mu.Lock() | ||
| defer e.mu.Unlock() | ||
|
|
||
| ecosystem := ev.Ecosystem | ||
| if ecosystem == "" { | ||
| ecosystem = ev.Artifact.Product | ||
| } | ||
| if ecosystem == "" { | ||
| ecosystem = "unknown" | ||
| } | ||
|
|
||
| stableID := fmt.Sprintf("min-package-age-suppressed-%s", ecosystem) | ||
| title := fmt.Sprintf("%s package versions suppressed", ecosystem) | ||
|
|
||
| for i := range e.events { | ||
| if e.events[i].ID == stableID { | ||
| e.events[i].TsMs = ev.TsMs | ||
| return e.events[i] | ||
| } | ||
| } | ||
|
|
||
| stored := MinPackageAgeEvent{ | ||
| ID: stableID, | ||
| TsMs: ev.TsMs, | ||
| Ecosystem: ecosystem, | ||
| Title: title, | ||
| Message: "One or more package versions were suppressed because they did not meet the minimum package age policy.", | ||
| } | ||
|
|
||
| e.events = append(e.events, stored) | ||
|
|
||
| return stored | ||
| } | ||
|
|
||
| func (e *minPackageAgeEventStore) Get(id string) (MinPackageAgeEvent, bool) { | ||
| e.mu.RLock() | ||
| defer e.mu.RUnlock() | ||
| for _, ev := range e.events { | ||
| if ev.ID == id { | ||
| return ev, true | ||
| } | ||
| } | ||
| return MinPackageAgeEvent{}, false | ||
| } | ||
|
|
||
| func (e *minPackageAgeEventStore) List() []MinPackageAgeEvent { | ||
| e.mu.RLock() | ||
| defer e.mu.RUnlock() | ||
| out := make([]MinPackageAgeEvent, len(e.events)) | ||
| copy(out, e.events) | ||
| return out | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package ingress | ||
|
|
||
| import "testing" | ||
|
|
||
| func TestMinPackageAgeEventStoreAddAssignsIDAndCopiesVersions(t *testing.T) { | ||
| store := &minPackageAgeEventStore{} | ||
|
|
||
| input := MinPackageAgeEvent{ | ||
| TsMs: 123, | ||
| Ecosystem: "vscode", | ||
| } | ||
|
|
||
| stored := store.Add(input) | ||
| if stored.ID != "min-package-age-suppressed-vscode" { | ||
| t.Fatalf("expected stable id %q, got %q", "min-package-age-suppressed-vscode", stored.ID) | ||
| } | ||
| if stored.Title != "vscode package versions suppressed" || stored.Message == "" { | ||
| t.Fatalf("expected generic title and message to be populated") | ||
| } | ||
| if stored.Ecosystem != "vscode" { | ||
| t.Fatalf("expected ecosystem to be stored, got %q", stored.Ecosystem) | ||
| } | ||
|
|
||
| got, ok := store.Get(stored.ID) | ||
| if !ok { | ||
| t.Fatalf("expected stored event to exist") | ||
| } | ||
| if got.Title != stored.Title || got.Message != stored.Message { | ||
| t.Fatalf("expected stored event to preserve generic copy") | ||
| } | ||
| } | ||
|
|
||
| func TestMinPackageAgeEventStoreAddUpdatesExistingEntryInsteadOfDuplicating(t *testing.T) { | ||
| store := &minPackageAgeEventStore{} | ||
|
|
||
| first := store.Add(MinPackageAgeEvent{TsMs: 123, Ecosystem: "vscode"}) | ||
| second := store.Add(MinPackageAgeEvent{TsMs: 456, Ecosystem: "vscode"}) | ||
|
|
||
| if first.ID != second.ID { | ||
| t.Fatalf("expected stable aggregate id, got %q and %q", first.ID, second.ID) | ||
| } | ||
| if len(store.List()) != 1 { | ||
| t.Fatalf("expected a single aggregate event, got %d", len(store.List())) | ||
| } | ||
| if second.TsMs != 456 { | ||
| t.Fatalf("expected timestamp to be refreshed, got %d", second.TsMs) | ||
| } | ||
| } | ||
|
|
||
| func TestMinPackageAgeEventStoreAddCreatesOneEntryPerEcosystem(t *testing.T) { | ||
| store := &minPackageAgeEventStore{} | ||
|
|
||
| first := store.Add(MinPackageAgeEvent{TsMs: 123, Ecosystem: "vscode"}) | ||
| second := store.Add(MinPackageAgeEvent{TsMs: 456, Ecosystem: "npm"}) | ||
|
|
||
| if first.ID == second.ID { | ||
| t.Fatalf("expected different ids per ecosystem, got %q", first.ID) | ||
| } | ||
| if len(store.List()) != 2 { | ||
| t.Fatalf("expected one aggregate event per ecosystem, got %d", len(store.List())) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -195,6 +195,49 @@ func GetTlsEvent(eventID string) (TlsTerminationFailedEvent, error) { | |
| return out, nil | ||
| } | ||
|
|
||
| // ListMinPackageAgeEvents fetches GET /v1/min-package-age-events?limit=N. | ||
|
Contributor
There was a problem hiding this comment. ListMinPackageAgeEvents and GetMinPackageAgeEvent duplicate the request/response handling logic already used by ListTlsEvents/GetTlsEvent; consider extracting a shared helper to avoid repeating nearly identical code. Details✨ AI Reasoning 🔧 How do I fix it? Reply |
||
| func ListMinPackageAgeEvents(limit int) ([]MinPackageAgeEvent, error) { | ||
| if limit <= 0 { | ||
| limit = 50 | ||
| } | ||
| resp, err := doRequest(http.MethodGet, fmt.Sprintf("/v1/min-package-age-events?limit=%d", limit), nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("list min package age events: %s", resp.Status) | ||
| } | ||
| var out []MinPackageAgeEvent | ||
| if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { | ||
| return nil, err | ||
| } | ||
| sort.Slice(out, func(i, j int) bool { | ||
| return out[i].TsMs > out[j].TsMs | ||
| }) | ||
| return out, nil | ||
| } | ||
|
|
||
| // GetMinPackageAgeEvent fetches GET /v1/min-package-age-events/:id. | ||
| func GetMinPackageAgeEvent(eventID string) (MinPackageAgeEvent, error) { | ||
| var out MinPackageAgeEvent | ||
| if err := validateEventID(eventID); err != nil { | ||
| return out, err | ||
| } | ||
| resp, err := doRequest(http.MethodGet, "/v1/min-package-age-events/"+eventID, nil) | ||
| if err != nil { | ||
| return out, err | ||
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode != http.StatusOK { | ||
| return out, fmt.Errorf("get min package age event: %s", resp.Status) | ||
| } | ||
| if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { | ||
| return out, err | ||
| } | ||
| return out, nil | ||
| } | ||
|
|
||
| // CertificateStatus is returned by GET /v1/certificate/status. | ||
| type CertificateStatus struct { | ||
| NeedsInstall bool `json:"needs_install"` | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleMinPackageAge duplicates the token-validate/decode/validate/callback pattern in other handlers; extract common request-handling steps into a shared helper to avoid repeating the same logic.
Details
✨ AI Reasoning
A new HTTP handler was added that performs the same sequence of steps as several existing handlers: validate token, decode JSON into a typed event, call Validate(), acquire lock to fetch a callback field, invoke the callback if present, and return HTTP 200. This repeats substantial, non-trivial logic already implemented in other handler functions in the same module, increasing maintenance burden because bug fixes or behavioral changes would need to be applied in multiple places. Consolidating the shared flow into a helper or common handler would reduce duplication and the risk of inconsistent behavior.
🔧 How do I fix it?
Delete extra code. Extract repeated code sequences into reusable functions or methods. Use loops or data structures to eliminate repetitive patterns.
Reply
@AikidoSec feedback: [FEEDBACK]to get better review comments in the future.Reply
@AikidoSec ignore: [REASON]to ignore this issue.More info