Skip to content

Fix support for `runnerToken`, and prevent setting deprecated environment variables when using an external secret controller to inject an authentication token instead of passing the value in via helm

What does this MR do?

Makes helper isAuthToken return true if values.yaml runnerRegistrationToken is explicitly defined as an empty string, or if runnerToken is defined and not an empty string. This provides a method for users to avoid confusing behavior where the deprecated variables RUNNER_TAGS_LIST and REGISTER_LOCKED are set on the deployment and unset by the registration script, which occurs when a user doesn't define either runnerRegistrationToken or runnerToken in values.yaml.

Why was this MR needed?

A user may choose to leave both runnerRegistrationToken and runnerToken unset in order to pass the secret into the environment via an external secrets controller. When passing an authentication token outside of helm via such a controller, the chart has no knowledge of this and sets the deprecated environment variables RUNNER_TAG_LIST and REGISTER_LOCKED on the container manifest, while the registration script unsets them in the running container. This happens because the $isAuthToken check in its present form has insufficient logic to allow for this use-case.

Additionally, when defining runnerToken but not runnerRegistrationToken, the same behavior is exhibited because the same check ignores the value of runnerToken entirely.

Thirdly, when defining runnerRegistrationToken using an authentication token prefixed with glrt- as the value, the chart does not define the 2 above environment variables in the container spec, but still attempts to unset them in the running container registration script.

Since the 2 above mentioned environment variables are both deprecated, they should not be set in the deployment manifest except for if a legacy registration token is passed, because the runners registered by authentication token are supposed to get these values from the settings registered to the runner via the UI. It's confusing to someone trying to migrate from registration tokens to authentication tokens as to which values are going to be used if the runners are using an authentication token but have the environment variables defined on the deployment. Which values will be used? Those set in the deployment environment vars, or those set on the runner the user registered in the UI?

Finally, this removes an unnecessary conditional.

  • To work around the first issue, a user may now explicitly set runnerRegistrationToken to an empty string to force isAuthToken to return true without defining runnerToken, so that these variables are not created in the container spec.
  • To work around the second issue, isAuthToken now returns true if runnerToken is defined and not an empty string
  • To work around the third issue, the chart now avoids rendering the 2 unset commands if isAuthToken returns false.

What's the best way to test this MR?

The below test plan provides a method to test whether this is a breaking change or not. This merge request should NOT cause a breaking change; runners registered with non-authentication registration token passed via --set runnerRegistrationToken will continue to set these variables while runners registered in the UI and installed with --set runnerToken set to a valid authentication token will no longer set these variables, and neither will runners registered in the UI and installed with --set runnerRegistrationToken="" which is something one might do if using an external secrets controller as mentioned before..

  • In main: helm template gitlab-runner ., then observe that the configmap spec value register-the-runner contains the lines unset REGISTER_LOCKED and unset RUNNER_TAG_LIST and these values are also being set in the deployment spec.
  • Switch to this branch and run the same command, then observe that the lines are still present, representing no change to the default behavior.
  • Switch back to main and run: helm template gitlab-runner . --set runnerToken=glrt-foobar, then observe the same behavior as mentioned before despite the runner token being properly passed in.
  • Switch back to this branch and run the same command, then observe that the deprecated variables and the related unset commands have been removed from the chart.
  • Switch over to main again and run: helm template gitlab-runner . --set runnerRegistrationToken=glrt-foobar, then observe that this time, the unset commands are present, but the variables are not set on the container.
  • Switch over to this branch again and run the same command as before, then observe that the unset lines are removed.
  • Switch another time to the main branch and run: helm template gitlab-runner . --set runnerRegistrationToken=foobar (note, not prefixed with "glrt-"); observe that the unset commands and variables are back
  • Switch another time to this branch and run the same command, then observe that both the unset lines and variables are still present as expected
  • Switch back to main one last time and run: helm template gitlab-runner . --set runnerRegistrationToken="", then observe that both the variables and the unset commands are present again
  • Switch back to this branch a final time and run the same command, then observe that the unset commands and variables are again removed.

What are the relevant issue numbers?

Edited by Thomas Spear

Merge request reports