Skip to content

Commit 91e9664

Browse files
davissp14davissp14
authored
Deepsource fixes (#111)
* Various fixes and cleanup * Add deepsource toml * Add version * Fix anti-pattern * Remove used slice * clean up handlers * remove unused param * fix * Remove unused receiver * no need to exit here * Don't exit in main * Cleanup unregistration script * Go security fixes * Fixing another go security issue * Couple more fixes * More cleanup * Make naming less confusing * more cleanup * Remove usage of ioutil * Short circuit * Return err * Fix handler bug * If clone fails, remove postgresql directory so it can be retried * Ensure password file gets the correct permissions * Ensure perms are correct pre-clone
1 parent 2f40d2d commit 91e9664

24 files changed

+422
-339
lines changed

‎.deepsource.toml

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
version = 1
2+
3+
[[analyzers]]
4+
name = "shell"
5+
6+
[[analyzers]]
7+
name = "go"
8+
9+
[analyzers.meta]
10+
import_root = "github.com/fly-apps/postgres-flex"

‎cmd/admin_server/main.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,10 @@ package main
22

33
import (
44
"github.com/fly-apps/postgres-flex/internal/api"
5-
"github.com/fly-apps/postgres-flex/internal/flypg"
65
)
76

87
func main() {
9-
node, err := flypg.NewNode()
10-
if err != nil {
8+
if err := api.StartHttpServer(); err != nil {
119
panic(err)
1210
}
13-
14-
api.StartHttpServer(node)
1511
}

‎cmd/event_handler/main.go

+19-18
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,24 @@ import (
1515
const eventLogFile = "/data/event.log"
1616

1717
func main() {
18+
ctx := context.Background()
19+
20+
if err := processEvent(ctx); err != nil {
21+
log.Println(err)
22+
os.Exit(1)
23+
}
24+
}
25+
26+
func processEvent(ctx context.Context) error {
1827
event := flag.String("event", "", "event type")
1928
nodeID := flag.Int("node-id", 0, "the node id")
2029
success := flag.String("success", "", "success (1) failure (0)")
2130
details := flag.String("details", "", "details")
2231
flag.Parse()
2332

24-
ctx := context.Background()
25-
26-
logFile, err := os.OpenFile(eventLogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
33+
logFile, err := os.OpenFile(eventLogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600)
2734
if err != nil {
28-
fmt.Printf("failed to open event log: %s", err)
35+
return fmt.Errorf("failed to open event log: %s", err)
2936
}
3037
defer logFile.Close()
3138

@@ -34,46 +41,40 @@ func main() {
3441

3542
node, err := flypg.NewNode()
3643
if err != nil {
37-
log.Printf("failed to initialize node: %s", err)
38-
os.Exit(1)
44+
return fmt.Errorf("failed to initialize node: %s", err)
3945
}
4046

4147
switch *event {
4248
case "child_node_disconnect", "child_node_reconnect", "child_node_new_connect":
4349
conn, err := node.RepMgr.NewLocalConnection(ctx)
4450
if err != nil {
45-
log.Printf("failed to open local connection: %s", err)
46-
os.Exit(1)
51+
return fmt.Errorf("failed to open local connection: %s", err)
4752
}
4853
defer conn.Close(ctx)
4954

5055
member, err := node.RepMgr.Member(ctx, conn)
5156
if err != nil {
52-
log.Printf("failed to resolve member: %s", err)
53-
os.Exit(1)
57+
return fmt.Errorf("failed to resolve member: %s", err)
5458
}
5559

5660
if member.Role != flypg.PrimaryRoleName {
5761
// We should never get here.
5862
log.Println("skipping since we are not the primary")
59-
os.Exit(0)
63+
return nil
6064
}
6165

6266
if err := evaluateClusterState(ctx, conn, node); err != nil {
63-
log.Printf("failed to evaluate cluster state: %s", err)
64-
os.Exit(0)
67+
return fmt.Errorf("failed to evaluate cluster state: %s", err)
6568
}
66-
67-
os.Exit(0)
68-
default:
69-
// noop
7069
}
70+
71+
return nil
7172
}
7273

7374
func evaluateClusterState(ctx context.Context, conn *pgx.Conn, node *flypg.Node) error {
7475
primary, err := flypg.PerformScreening(ctx, conn, node)
7576
if errors.Is(err, flypg.ErrZombieDiagnosisUndecided) || errors.Is(err, flypg.ErrZombieDiscovered) {
76-
if err := flypg.Quarantine(ctx, conn, node, primary); err != nil {
77+
if err := flypg.Quarantine(ctx, node, primary); err != nil {
7778
return fmt.Errorf("failed to quarantine failed primary: %s", err)
7879
}
7980
return fmt.Errorf("primary has been quarantined: %s", err)

‎cmd/monitor/monitor_cluster_state.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func clusterStateMonitorTick(ctx context.Context, node *flypg.Node) error {
3939

4040
primary, err := flypg.PerformScreening(ctx, conn, node)
4141
if errors.Is(err, flypg.ErrZombieDiagnosisUndecided) || errors.Is(err, flypg.ErrZombieDiscovered) {
42-
if err := flypg.Quarantine(ctx, conn, node, primary); err != nil {
42+
if err := flypg.Quarantine(ctx, node, primary); err != nil {
4343
return fmt.Errorf("failed to quarantine failed primary: %s", err)
4444
}
4545
return fmt.Errorf("primary has been quarantined: %s", err)

‎cmd/monitor/monitor_dead_members.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func deadMemberMonitorTick(ctx context.Context, node *flypg.Node, seenAt map[int
7878
// TODO - Verify the exception that's getting thrown.
7979
if time.Since(seenAt[standby.ID]) >= deadMemberRemovalThreshold {
8080
log.Printf("Removing dead member: %s\n", standby.Hostname)
81-
if err := node.RepMgr.UnregisterMember(ctx, standby); err != nil {
81+
if err := node.RepMgr.UnregisterMember(standby); err != nil {
8282
log.Printf("failed to unregister member %s: %v", standby.Hostname, err)
8383
continue
8484
}

‎cmd/pg_unregister/main.go

+18-19
Original file line numberDiff line numberDiff line change
@@ -11,43 +11,42 @@ import (
1111
)
1212

1313
func main() {
14+
ctx := context.Background()
15+
16+
if err := processUnregistration(ctx); err != nil {
17+
utils.WriteError(err)
18+
os.Exit(1)
19+
}
20+
21+
utils.WriteOutput("Member has been succesfully unregistered", "")
22+
}
23+
24+
func processUnregistration(ctx context.Context) error {
1425
encodedArg := os.Args[1]
1526
hostnameBytes, err := base64.StdEncoding.DecodeString(encodedArg)
1627
if err != nil {
17-
utils.WriteError(fmt.Errorf("failed to decode hostname: %v", err))
18-
os.Exit(1)
19-
return
28+
return fmt.Errorf("failed to decode hostname: %v", err)
2029
}
2130

22-
ctx := context.Background()
23-
2431
node, err := flypg.NewNode()
2532
if err != nil {
26-
utils.WriteError(err)
27-
os.Exit(1)
28-
return
33+
return fmt.Errorf("faied to initialize node: %s", err)
2934
}
3035

3136
conn, err := node.RepMgr.NewLocalConnection(ctx)
3237
if err != nil {
33-
utils.WriteError(fmt.Errorf("failed to connect to local db: %s", err))
34-
os.Exit(1)
35-
return
38+
return fmt.Errorf("failed to connect to local db: %s", err)
3639
}
3740
defer conn.Close(ctx)
3841

3942
member, err := node.RepMgr.MemberByHostname(ctx, conn, string(hostnameBytes))
4043
if err != nil {
41-
utils.WriteError(fmt.Errorf("failed to resolve member: %s", err))
42-
os.Exit(1)
43-
return
44+
return fmt.Errorf("failed to resolve member: %s", err)
4445
}
4546

46-
if err := node.RepMgr.UnregisterMember(ctx, *member); err != nil {
47-
utils.WriteError(fmt.Errorf("failed to unregister member: %v", err))
48-
os.Exit(1)
49-
return
47+
if err := node.RepMgr.UnregisterMember(*member); err != nil {
48+
return fmt.Errorf("failed to unregister member: %v", err)
5049
}
5150

52-
utils.WriteOutput("Member has been succesfully unregistered", "")
51+
return nil
5352
}

‎internal/api/handle_admin.go

+44-21
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
"golang.org/x/exp/slices"
1313
)
1414

15-
func handleReadonlyState(w http.ResponseWriter, r *http.Request) {
15+
func handleReadonlyState(w http.ResponseWriter, _ *http.Request) {
1616
res := &Response{
1717
Result: false,
1818
}
@@ -24,7 +24,7 @@ func handleReadonlyState(w http.ResponseWriter, r *http.Request) {
2424
renderJSON(w, res, http.StatusOK)
2525
}
2626

27-
func handleHaproxyRestart(w http.ResponseWriter, r *http.Request) {
27+
func handleHaproxyRestart(w http.ResponseWriter, _ *http.Request) {
2828
if err := flypg.RestartHaproxy(); err != nil {
2929
renderErr(w, err)
3030
return
@@ -78,18 +78,18 @@ func handleDisableReadonly(w http.ResponseWriter, r *http.Request) {
7878
}
7979

8080
func handleRole(w http.ResponseWriter, r *http.Request) {
81-
conn, close, err := localConnection(r.Context(), "postgres")
81+
node, err := flypg.NewNode()
8282
if err != nil {
8383
renderErr(w, err)
8484
return
8585
}
86-
defer close()
8786

88-
node, err := flypg.NewNode()
87+
conn, err := localConnection(r.Context(), "postgres")
8988
if err != nil {
9089
renderErr(w, err)
9190
return
9291
}
92+
defer conn.Close(r.Context())
9393

9494
member, err := node.RepMgr.Member(r.Context(), conn)
9595
if err != nil {
@@ -118,21 +118,27 @@ type SettingsUpdate struct {
118118
RestartRequired bool `json:"restart_required"`
119119
}
120120

121-
func (s *Server) handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Request) {
122-
conn, close, err := localConnection(r.Context(), "postgres")
121+
func handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Request) {
122+
node, err := flypg.NewNode()
123123
if err != nil {
124124
renderErr(w, err)
125125
return
126126
}
127-
defer close()
127+
128+
conn, err := localConnection(r.Context(), "postgres")
129+
if err != nil {
130+
renderErr(w, err)
131+
return
132+
}
133+
defer conn.Close(r.Context())
128134

129135
consul, err := state.NewStore()
130136
if err != nil {
131137
renderErr(w, err)
132138
return
133139
}
134140

135-
user, err := flypg.ReadFromFile(s.node.PGConfig.UserConfigFile())
141+
user, err := flypg.ReadFromFile(node.PGConfig.UserConfigFile())
136142
if err != nil {
137143
renderErr(w, err)
138144
return
@@ -158,7 +164,7 @@ func (s *Server) handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Req
158164
user[k] = v
159165
}
160166

161-
s.node.PGConfig.SetUserConfig(user)
167+
node.PGConfig.SetUserConfig(user)
162168

163169
var requiresRestart []string
164170

@@ -185,7 +191,7 @@ func (s *Server) handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Req
185191
}}
186192
}
187193

188-
err = flypg.PushUserConfig(s.node.PGConfig, consul)
194+
err = flypg.PushUserConfig(node.PGConfig, consul)
189195
if err != nil {
190196
renderErr(w, err)
191197
return
@@ -194,21 +200,27 @@ func (s *Server) handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Req
194200
renderJSON(w, res, http.StatusOK)
195201
}
196202

197-
func (s *Server) handleApplyConfig(w http.ResponseWriter, r *http.Request) {
198-
conn, close, err := localConnection(r.Context(), "postgres")
203+
func handleApplyConfig(w http.ResponseWriter, r *http.Request) {
204+
node, err := flypg.NewNode()
199205
if err != nil {
200206
renderErr(w, err)
201207
return
202208
}
203-
defer close()
209+
210+
conn, err := localConnection(r.Context(), "postgres")
211+
if err != nil {
212+
renderErr(w, err)
213+
return
214+
}
215+
defer conn.Close(r.Context())
204216

205217
consul, err := state.NewStore()
206218
if err != nil {
207219
renderErr(w, err)
208220
return
209221
}
210222

211-
err = flypg.SyncUserConfig(s.node.PGConfig, consul)
223+
err = flypg.SyncUserConfig(node.PGConfig, consul)
212224
if err != nil {
213225
renderErr(w, err)
214226
return
@@ -225,16 +237,21 @@ type PGSettingsResponse struct {
225237
Settings []admin.PGSetting `json:"settings"`
226238
}
227239

228-
func (s *Server) handleViewPostgresSettings(w http.ResponseWriter, r *http.Request) {
229-
conn, close, err := localConnection(r.Context(), "postgres")
240+
func handleViewPostgresSettings(w http.ResponseWriter, r *http.Request) {
241+
node, err := flypg.NewNode()
230242
if err != nil {
231243
renderErr(w, err)
232244
return
233245
}
234246

235-
defer close()
247+
conn, err := localConnection(r.Context(), "postgres")
248+
if err != nil {
249+
renderErr(w, err)
250+
return
251+
}
252+
defer conn.Close(r.Context())
236253

237-
all, err := s.node.PGConfig.CurrentConfig()
254+
all, err := node.PGConfig.CurrentConfig()
238255
if err != nil {
239256
renderErr(w, err)
240257
return
@@ -264,8 +281,14 @@ func (s *Server) handleViewPostgresSettings(w http.ResponseWriter, r *http.Reque
264281
renderJSON(w, resp, http.StatusOK)
265282
}
266283

267-
func (s *Server) handleViewRepmgrSettings(w http.ResponseWriter, r *http.Request) {
268-
all, err := s.node.RepMgr.CurrentConfig()
284+
func handleViewRepmgrSettings(w http.ResponseWriter, r *http.Request) {
285+
node, err := flypg.NewNode()
286+
if err != nil {
287+
renderErr(w, err)
288+
return
289+
}
290+
291+
all, err := node.RepMgr.CurrentConfig()
269292
if err != nil {
270293
renderErr(w, err)
271294
return

0 commit comments

Comments
 (0)