Setting up CI/CD

[trezy]

TL;DR

I think we should set up CI/CD. @Ariel-Thomas, you're obviously welcome to do it yourself. For me to dig into it, though, I'd have to be added as a contributor on the repo and added to the team on Heroku. I can get the CircleCI stuff set up from scratch and add you to the team.

The long version

As far as I'm aware, we don't have any form of CI/CD set up at the moment. I'd be happy to dive in and make that a reality to shorten the time it takes for features to go from development to production. Here's the setup I usually use, adapted for the way that I think our infrastructure is currently set up:

Continuous Integration via CircleCI

CircleCI has a super robust framework for CI that I absolutely adore. I usually set up CircleCI to run builds and tests on every commit to the website. This gets status checks showing up on PRs and such, which means we don't have to run the tests manually before merging a PR.

Continuous Deployment with CircleCI and Heroku

I think we already deploy to Heroku. If that's the case, CircleCI and Heroku make deployments really simple. When anything gets merged to the develop branch, it gets deployed automatically to the development website (https://dev.adventurersleaguelog.com?), and the same thing happens for the master branch and the production website. The next section details how to make sure that automated deployments are still safe, though.

Locking down the repo

CI/CD is awesome, but with the amount of automation I tend to build in, it's not the best idea to leave it open. Here's my standard repo set up:

Feature branches

Most of our work already happens on feature branches, so this isn't anything new. The big difference is that if we move forward with the rest of these changes, it'll be an absolute necessity that all of our work happens on feature branches, as we won't be able to push directly to develop or master. These changes will be added to develop by PR.

develop branch

The develop branch is the source of truth for the development website. It's a bit more wild west than the master branch, but for the most part the develop branch should almost always be in a deployable state. This is where we start really locking things down, though.

I always set up develop as a "protected" branch. These are the settings I use.

1. Restricting who can push to the branch prevents anybody from being able to push directly to it. This mostly happens when people pull develop and forget to make a new branch before making their changes.
2. Requiring status checks to pass means that the tests will have to pass before we merge. That doesn't mean nothing broken will make it though, but it at least means that the parts of the site that have tests should be in the clear.
3. The purpose of requiring PR reviews is similar to restricting who can push to the branch. It prevents anybody from pushing a feature branch, creating a PR, and merging it without any oversight.

master branch

The master branch is the source of truth for the production website. It takes on all of the same properties of the develop branch for safety's sake. The biggest difference is that I actually pull down the code and make sure nothing's obviously broken before approving a PR for master. On develop, I just read through the code and check for code smells and obvious issues.

[pamapa]

Most probably a configuration problem on your IdP side and not on client side.

To fix: double check your IdP config...

[fpue]

I digged further into the code and I think it's these lines of code. Is there a reason for basic requiring a secret, while post doesn't?

https://github.com/authts/oidc-client-ts/blob/14168c006c3073be00580d8253f882dad36658f9/src/TokenClient.ts#L109C12-L115C23

[pamapa]

I digged further into the code and I think it's these lines of code. Is there a reason for basic requiring a secret, while post doesn't?

https://github.com/authts/oidc-client-ts/blob/14168c006c3073be00580d8253f882dad36658f9/src/TokenClient.ts#L109C12-L115C23

That code path is only taken if settings.client_authentication is set. Which you do, I oversight it the first time sorry. Do not set this for code flow with PKCE...

[fpue]

But my IdP only supports basic. Switching IdP is currently not an option, so this ticket now is more a feature request than a question.

Please add the ability to use client_authentication: 'client_secret_basic' and PKCE at the same time.

[pamapa]

But my IdP only supports basic.

Then you need a secret client side...