Skip to content

Commit

Permalink
Catch or explicitly ignore unhandled errors
Browse files Browse the repository at this point in the history
  • Loading branch information
geofffranks committed Sep 18, 2024
1 parent d3eb0f6 commit 460ea10
Show file tree
Hide file tree
Showing 20 changed files with 43 additions and 5 deletions.
5 changes: 4 additions & 1 deletion src/code.cloudfoundry.org/cni-wrapper-plugin/lib/lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ func LoadWrapperConfig(bytes []byte) (*WrapperConfig, error) {
return nil, fmt.Errorf("invalid outbound connection rate")
}

validator.Validate(n)
err := validator.Validate(n)
if err != nil {
return nil, fmt.Errorf("validator: %s", err)
}

return n, nil
}
Expand Down
4 changes: 3 additions & 1 deletion src/code.cloudfoundry.org/cni-wrapper-plugin/lib/lib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ var _ = Describe("LoadWrapperConfig", func() {
"burst": 900,
"rate_per_sec": 100,
"dry_run": false
}
},
"policy_agent_force_poll_address": "http://127.0.0.1:1234"
}`)
})

Expand All @@ -59,6 +60,7 @@ var _ = Describe("LoadWrapperConfig", func() {
NoMasqueradeCIDRRange: "10.255.0.0/16",
UnderlayIPs: []string{"10.244.20.1", "10.244.20.2"},
TemporaryUnderlayInterfaceNames: []string{"some-temporary-underlay-interface-name"},
PolicyAgentForcePollAddress: "http://127.0.0.1:1234",
IPTablesASGLogging: true,
Delegate: map[string]interface{}{
"cniVersion": "1.0.0",
Expand Down
3 changes: 3 additions & 0 deletions src/code.cloudfoundry.org/cni-wrapper-plugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func cmdAdd(args *skel.CmdArgs) error {
}
if resp.StatusCode != http.StatusOK {
body, _ := io.ReadAll(resp.Body)
// #nosec G104 - don't capture this error, as the one we generate below is more important to return
resp.Body.Close()
return fmt.Errorf("vpa response code: %v with message: %s", resp.StatusCode, body)
}
Expand Down Expand Up @@ -205,6 +206,7 @@ func cmdAdd(args *skel.CmdArgs) error {

if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusMethodNotAllowed {
body, _ := io.ReadAll(resp.Body)
// #nosec G104 - don't capture this error, as the one we generate below is more important to return
resp.Body.Close()
return fmt.Errorf("asg sync returned %v with message: %s", resp.StatusCode, body)
}
Expand Down Expand Up @@ -337,6 +339,7 @@ func cmdDel(args *skel.CmdArgs) error {
}
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusMethodNotAllowed {
body, _ := io.ReadAll(resp.Body)
// #nosec G104 - don't capture this error, as the one we generate below is more important to return
resp.Body.Close()
return fmt.Errorf("asg cleanup returned %v with message: %s", resp.StatusCode, body)
}
Expand Down
5 changes: 4 additions & 1 deletion src/code.cloudfoundry.org/netmon/cmd/netmon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ func main() {
IPTablesRunner: iptablesCommandRunner,
}

dropsonde.Initialize(conf.MetronAddress, "netmon")
err = dropsonde.Initialize(conf.MetronAddress, "netmon")
if err != nil {
logger.Fatal("failed-initializing-dropsonde", err)
}

networkStatsFetcher := network_stats.NewFetcher(lockedIPTables, logger)
ruleCountAggregator := network_stats.NewIntAggregator()
Expand Down
2 changes: 1 addition & 1 deletion src/code.cloudfoundry.org/silk-daemon-bootstrap/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func mainWithError() error {
}

func createNewChain(ipTablesAdapter rules.IPTablesAdapter) error {
// NewChain only returns an error if the chain already exists, so we ignore it :(
// #nosec G104 - NewChain only returns an error if the chain already exists, so we ignore it :(
ipTablesAdapter.NewChain("filter", IngressChainName)

jumpRule := rules.IPTablesRule{
Expand Down
5 changes: 4 additions & 1 deletion src/code.cloudfoundry.org/silk-daemon-shutdown/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ func flushAndDeleteChain(lockedIPTables *rules.LockedIPTables) error {
}
exists, _ := lockedIPTables.Exists("filter", "OUTPUT", jumpRule)
if exists {
lockedIPTables.Delete("filter", "OUTPUT", jumpRule)
err := lockedIPTables.Delete("filter", "OUTPUT", jumpRule)
if err != nil {
return err
}
}

err := lockedIPTables.ClearChain("filter", "istio-ingress")
Expand Down
1 change: 1 addition & 0 deletions src/code.cloudfoundry.org/silk/cmd/silk-daemon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ func buildHealthCheckServer(healthCheckPort uint16, networkInfo daemon.NetworkIn
fmt.Sprintf("127.0.0.1:%d", healthCheckPort),
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write(networkBytes)
}),
), nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func main() {
fmt.Sprintf("127.0.0.1:%s", port),
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(code)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(response))
}),
)
Expand Down
1 change: 1 addition & 0 deletions src/code.cloudfoundry.org/silk/cni/lib/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func (s *Common) BasicSetup(deviceName string, local, peer config.DualAddress) e
s.Logger.Debug("hardware-addr-set-correctly", lager.Data{"addr": l.Attrs().HardwareAddr.String()})
}

// #nosec G104 - we have tests explicitly checking that we ignore failures here, so don't handle it
s.LinkOperations.DisableIPv6(deviceName)

if err := s.LinkOperations.StaticNeighborNoARP(link, peer.IP, peer.Hardware); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ func (l *LeasesAcquire) ServeHTTP(logger lager.Logger, w http.ResponseWriter, re
return
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write(bytes)
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@ func (l *LeasesIndex) ServeHTTP(logger lager.Logger, w http.ResponseWriter, req
return
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write(bytes)
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ func (l *ReleaseLease) ServeHTTP(logger lager.Logger, w http.ResponseWriter, req
return
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(`{}`))
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,6 @@ func (l *RenewLease) ServeHTTP(logger lager.Logger, w http.ResponseWriter, req *
return
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte("{}"))
}
2 changes: 2 additions & 0 deletions src/code.cloudfoundry.org/silk/testsupport/fakecontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func (f *FakeController) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
if fakeHandler == nil {
w.WriteHeader(http.StatusTeapot)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(`{}`))
return
}
Expand All @@ -61,6 +62,7 @@ func (f *FakeController) ServeHTTP(w http.ResponseWriter, r *http.Request) {
fakeHandler.LastRequestBody = bodyBytes
responseBytes, _ := json.Marshal(fakeHandler.ResponseBody)
w.WriteHeader(fakeHandler.ResponseCode)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write(responseBytes)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func (f *FakePolicyServer) Start(listenAddr string, tlsConfig *tls.Config) {
switch r.URL.Path {
case "/networking/v1/internal/tags":
w.WriteHeader(http.StatusOK)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(fmt.Sprintf(`{
"id": "some-id",
"type": "some-type",
Expand Down
2 changes: 2 additions & 0 deletions src/code.cloudfoundry.org/testsupport/fakecontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@ func (f *FakeController) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
if fakeHandler == nil {
w.WriteHeader(http.StatusTeapot)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(`{}`))
return
}

fakeHandler.LastRequestBody = bodyBytes
responseBytes, _ := json.Marshal(fakeHandler.ResponseBody)
w.WriteHeader(fakeHandler.ResponseCode)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write(responseBytes)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,25 @@ type ForceASGsForContainer struct {
func (h *ForceASGsForContainer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if !h.EnableASGSyncing {
w.WriteHeader(http.StatusMethodNotAllowed)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte("ASG syncing has been disabled administratively"))
return
}

container := r.URL.Query().Get("container")
if container == "" {
w.WriteHeader(http.StatusBadRequest)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte("no container specified"))
return
}
if err := h.ASGUpdateFunc(container); err != nil {
errorMessage := fmt.Sprintf("failed to update asgs for container %s: %s", container, err)
w.WriteHeader(http.StatusInternalServerError)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(errorMessage))
return
}
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(fmt.Sprintf("updated container %s", container)))
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,26 @@ type ForceOrphanedASGsCleanup struct {
func (h *ForceOrphanedASGsCleanup) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if !h.EnableASGSyncing {
w.WriteHeader(http.StatusMethodNotAllowed)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte("ASG syncing has been disabled administratively"))
return
}

container := r.URL.Query().Get("container")
if container == "" {
w.WriteHeader(http.StatusBadRequest)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte("no container specified"))
return
}
if err := h.ASGCleanupFunc(container); err != nil {
errorMessage := fmt.Sprintf("failed to cleanup ASGs for container %s: %s", container, err)
w.WriteHeader(http.StatusInternalServerError)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(errorMessage))
return
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(fmt.Sprintf("cleaned up ASGs for container %s", container)))
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func (h *ForcePolicyPollCycle) ServeHTTP(w http.ResponseWriter, r *http.Request)
if err := h.PollCycleFunc(); err != nil {
errorMessage := fmt.Sprintf("failed to force policy poll cycle: %s", err)
w.WriteHeader(http.StatusInternalServerError)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(errorMessage))
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ func (h *IPTablesLogging) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err := json.NewDecoder(r.Body).Decode(&bodyStruct)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(`{ "error": "decoding request body as json" }`))
return
}
if bodyStruct.Enabled == nil {
w.WriteHeader(http.StatusBadRequest)
// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
w.Write([]byte(`{ "error": "missing required key 'enabled'" }`))
return
}
Expand All @@ -40,6 +42,7 @@ func (h *IPTablesLogging) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}

// #nosec G104 - ignore errors when writing HTTP responses so we don't spam our logs during a DoS
json.NewEncoder(w).Encode(struct {
Enabled bool `json:"enabled"`
}{h.LoggingState.IsEnabled()})
Expand Down

0 comments on commit 460ea10

Please sign in to comment.