Skip to content

Conversation

@pseudomorph
Copy link
Contributor

what

Allow markdown templates to be re-loaded without server restart. Feature is behind a flag.

why

Changing templates shouldn't require a full restart.

tests

Added tests to validate functionality

references

@github-actions github-actions bot added the go Pull requests that update Go code label Sep 15, 2025
@dosubot dosubot bot added the feature New functionality/enhancement label Sep 15, 2025
Signed-off-by: Ross Strickland <[email protected]>
@github-actions github-actions bot added the docs Documentation label Sep 15, 2025
@lukemassa
Copy link
Contributor

I believe this will cause a race condition.

I wrote a quick test:

func TestMarkdownRendererRace(t *testing.T) {

	r := events.NewMarkdownRenderer(
		false,      // gitlabSupportsCommonMark
		false,      // disableApplyAll
		false,      // disableApply
		false,      // disableMarkdownFolding
		false,      // disableRepoLocking
		false,      // enableDiffMarkdownFormat
		"",         // markdownTemplateOverridesDir
		"atlantis", // executableName
		false,      // hideUnchangedPlanComments
		false,      // quietPolicyChecks
		true,       // liveReloadEnabled
	)
	logger := logging.NewNoopLogger(t).WithHistory()
	logText := "log"
	logger.Info(logText)
	ctx := &command.Context{
		Log: logger,
		Pull: models.PullRequest{
			BaseRepo: models.Repo{
				VCSHost: models.VCSHost{
					Type: models.Github,
				},
			},
		},
	}

	var wg sync.WaitGroup
	for i := 0; i < 50; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			res := command.Result{
				Error: errors.New("error"),
			}
			cmd := &events.CommentCommand{
				Name: command.Apply,
			}
			r.Render(ctx, res, cmd)
		}()
	}
	wg.Wait()
}

And ran it with go's race detector:

atlantis % go test -race  ./server/events -run TestMarkdownRendererRace

It showed that if multiple goroutines call Render at the same time (which could easily happen), there will be a data race and they might see incomplete data and blow up.

==================
WARNING: DATA RACE
Write at 0x00c000113288 by goroutine 23:
  github.com/runatlantis/atlantis/server/events.(*MarkdownRenderer).reloadTemplates()
      /Users/lmassa/atlantis/server/events/markdown_renderer.go:192 +0x9c
  github.com/runatlantis/atlantis/server/events.(*MarkdownRenderer).Render()
      /Users/lmassa/atlantis/server/events/markdown_renderer.go:200 +0x64
  github.com/runatlantis/atlantis/server/events_test.TestMarkdownRendererRace.func1()
      /Users/lmassa/atlantis/server/events/markdown_renderer_test.go:153 +0x17c

Previous write at 0x00c000113288 by goroutine 66:
  github.com/runatlantis/atlantis/server/events.(*MarkdownRenderer).reloadTemplates()
      /Users/lmassa/atlantis/server/events/markdown_renderer.go:192 +0x9c
  github.com/runatlantis/atlantis/server/events.(*MarkdownRenderer).Render()
      /Users/lmassa/atlantis/server/events/markdown_renderer.go:200 +0x64
  github.com/runatlantis/atlantis/server/events_test.TestMarkdownRendererRace.func1()
      /Users/lmassa/atlantis/server/events/markdown_renderer_test.go:153 +0x17c

We'd need some sort of synchronization here, probably nothing fancier than just a lock around the update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation feature New functionality/enhancement go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants