Restore IRC Ident on Reconnect

After a connection loss on an IRC session with a ngIRCd, the
alertmanager-irc-relay was unable to reconnect. After some debugging,
the error's origin was the state tracking within the used goirc library.
When using an unidentified session, ngIRCd prefixes the user's ident
with a `~`. The state tracking registers this and keeps `~${NICK}` as
the current and the new ident for future reconnects. However, `~` is not
a valid char for the `<user>` part in the `USER` command, at least not
for ngIRCd.

To clarify this behaviour, take a look at the following log. First, the
initial connection is begin established correctly. Keep an eye on the
`USER` command being sent to the server.

> http.go:132: INFO Starting HTTP server
> irc.go:308: INFO Connected to IRC server, waiting to establish session
> connection.go:543: DEBUG -> NICK alertbot
> connection.go:543: DEBUG -> USER alertbot 12 * :Alertmanager IRC Relay
> connection.go:474: DEBUG <- :__SERVER__ 001 alertbot :Welcome to the Internet Relay Network alertbot!~alertbot@__IP__

Now, there was a network incident and the session needs to be recreated.

> connection.go:466: ERROR irc.recv(): read tcp __REDACTED__: read: connection timed out
> connection.go:577: INFO irc.Close(): Disconnected from server.
> irc.go:150: INFO Disconnected from IRC
> reconciler.go:129: INFO Channel #alerts monitor: context canceled while monitoring
> irc.go:300: INFO Connecting to IRC __SERVER__
> backoff.go:111: INFO Backoff for 0s starts
> backoff.go:114: INFO Backoff for 0s ends
> connection.go:390: INFO irc.Connect(): Connecting to __SERVER__.
> irc.go:308: INFO Connected to IRC server, waiting to establish session
> connection.go:543: DEBUG -> NICK alertbot
> connection.go:543: DEBUG -> USER ~alertbot 12 * :Alertmanager IRC Relay
> connection.go:474: DEBUG <- ERROR :Invalid user name
> connection.go:577: INFO irc.Close(): Disconnected from server.
> irc.go:150: INFO Disconnected from IRC
> irc.go:319: WARN Receiving a session down before the session is up, this is odd

This time, the used `user` part of the `USER` command has the prefixed
`~` and fails. However, without using `-debug` and taking a very close
look, this error can be missed very easy.

As the new ident is invalid, the alertmanager-irc-relay is now stuck in
an endless reconnection loop.

This fix is kind of straight forward and just checks if the ident has
changed before trying to reconnect. It might not be the prettiest
solution, but recreating the whole *irc.Config resulted in other bugs as
it was still referenced - even after being `Close`d.

irc.go

@@ -85,6 +85,12 @@ type IRCNotifier struct {
 	NickservName string
 	NickservIdentifyPatterns []string
 
+	// As the goirc library might alter the irc.Config created by makeGOIRCConfig,
+	// we might also want to keep a reference to the original Config to restore
+	// the desired state.
+	Config    *Config
+	IrcConfig *irc.Config
+
 	Client    *irc.Conn
 	AlertMsgs chan AlertMsg
 
@@ -124,6 +130,8 @@ func NewIRCNotifier(config *Config, alertMsgs chan AlertMsg, delayerMaker Delaye
 		NickPassword:             config.IRCNickPass,
 		NickservName:             config.NickservName,
 		NickservIdentifyPatterns: config.NickservIdentifyPatterns,
+		Config:                   config,
+		IrcConfig:                ircConfig,
 		Client:                   client,
 		AlertMsgs:                alertMsgs,
 		sessionUpSignal:          make(chan bool),
@@ -299,6 +307,11 @@ func (n *IRCNotifier) ConnectedPhase(ctx context.Context) {
 
 func (n *IRCNotifier) SetupPhase(ctx context.Context) {
 	if !n.Client.Connected() {
+		if n.IrcConfig.Me.Ident != n.Config.IRCNick {
+			logging.Debug("Restoring IRC nick from %s to %s", n.IrcConfig.Me.Ident, n.Config.IRCNick)
+			n.IrcConfig.Me.Ident = n.Config.IRCNick
+		}
+
 		logging.Info("Connecting to IRC %s", n.Client.Config().Server)
 		if ok := n.BackoffCounter.DelayContext(ctx); !ok {
 			return

irc_test.go

@@ -404,6 +404,68 @@ func TestReconnect(t *testing.T) {
 	}
 }
 
+func TestReconnectNickIdentChange(t *testing.T) {
+	server, port := makeTestServer(t)
+	config := makeTestIRCConfig(port)
+	notifier, _, ctx, cancel, stopWg := makeTestNotifier(t, config)
+
+	var testStep sync.WaitGroup
+
+	userHandler := func(conn *bufio.ReadWriter, line *irc.Line) error {
+		testStep.Done()
+		r := fmt.Sprintf(":example.com 001 %s :Welcome to the Internet Relay Network %s!~%s@example.com\n",
+			line.Args[0], line.Args[0], line.Args[0])
+		_, err := conn.WriteString(r)
+		return err
+	}
+	joinHandler := func(conn *bufio.ReadWriter, line *irc.Line) error {
+		testStep.Done()
+		return hJOIN(conn, line)
+	}
+	server.SetHandler("USER", userHandler)
+	server.SetHandler("JOIN", joinHandler)
+
+	testStep.Add(2)
+	go notifier.Run(ctx, stopWg)
+
+	// Wait until the pre-joined channel is seen.
+	testStep.Wait()
+
+	if ident := notifier.IrcConfig.Me.Ident; ident != "~foo" {
+		t.Errorf("IRC client failed to learn new nick ident. Using %s", ident)
+	}
+
+	// Simulate disconnection.
+	testStep.Add(2)
+	server.Client.Close()
+
+	// Wait again until the pre-joined channel is seen.
+	testStep.Wait()
+
+	cancel()
+	stopWg.Wait()
+
+	server.Stop()
+
+	expectedCommands := []string{
+		// Commands from first connection
+		"NICK foo",
+		"USER foo 12 * :",
+		"PRIVMSG ChanServ :UNBAN #foo",
+		"JOIN #foo",
+		// Commands from reconnection
+		"NICK foo",
+		"USER foo 12 * :", // NOTE: the client didn't used ~foo as its ident
+		"PRIVMSG ChanServ :UNBAN #foo",
+		"JOIN #foo",
+		"QUIT :see ya",
+	}
+
+	if !reflect.DeepEqual(expectedCommands, server.Log) {
+		t.Error("Reconnection did not happen correctly. Received commands:\n", strings.Join(server.Log, "\n"))
+	}
+}
+
 func TestConnectErrorRetry(t *testing.T) {
 	server, port := makeTestServer(t)
 	config := makeTestIRCConfig(port)