fix: add missing vcs-upload-url flag for enterprise github

[benjohnson-dev]

When trying to use kubechecks with enterprise github I get the following error:

error="failed to create vcs client: failed to get user: GET https://api.github.com/user: 401 Bad credentials []"

configMap:
  create: true
  env:
    KUBECHECKS_VCS_BASE_URL: https://<redacted>/
    KUBECHECKS_VCS_UPLOAD_URL: https://<redacted>/
    KUBECHECKS_VCS_TYPE: github

Meaning it is still trying to connect to github proper.

After checking the source, I believe the KUBECHECKS_VCS_UPLOAD_URL isn't being parsed, though is referenced in the Github client -

kubechecks/pkg/vcs/github_client/client.go

Line 64 in 35dfcf6

githubUploadUrl := cfg.VcsUploadUrl

I wasn't able to properly get the local testing environment fully working due to ngrok mitm issues, but I believe this should resolve the issue as I saw the 401 error disappear in the pod output.

[djeebus]

Nice catch, thanks!

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of cmd/root.go

@@ -50,6 +50,7 @@ func init() {
 	)
 	boolFlag(flags, "persist-log-level", "Persists the set log level down to other module loggers.")
 	stringFlag(flags, "vcs-base-url", "VCS base url, useful if self hosting gitlab, enterprise github, etc.")
+	stringFlag(flags, "vcs-upload-url", "VCS upload url, required for enterprise github.")
 	stringFlag(flags, "vcs-type", "VCS type. One of gitlab or github. Defaults to gitlab.",
 		newStringOpts().
 			withChoices("github", "gitlab").

Feedback & Suggestions:

1. Validation of URL: Ensure that the vcs-upload-url is validated to be a proper URL format. This can prevent potential issues with malformed URLs being used in the application. Consider using a URL parsing library to validate the input.

2. Security Consideration: If the vcs-upload-url is sensitive or could be used in a security context, ensure that it is not logged or exposed in error messages. This is crucial to prevent leaking sensitive information.

3. Documentation Update: Make sure to update any relevant documentation to include the new vcs-upload-url flag, explaining its purpose and usage, especially since it's required for enterprise GitHub.

4. Default Value: Consider if a default value is appropriate for vcs-upload-url, especially if there is a common default that could be used for most users. This can improve user experience by reducing the need for configuration.

---

😼 Mergecat review of docs/usage.md

@@ -68,6 +68,7 @@ The full list of supported environment variables is described below:
 |`KUBECHECKS_VCS_BASE_URL`|VCS base url, useful if self hosting gitlab, enterprise github, etc.||
 |`KUBECHECKS_VCS_TOKEN`|VCS API token.||
 |`KUBECHECKS_VCS_TYPE`|VCS type. One of gitlab or github.|`gitlab`|
+|`KUBECHECKS_VCS_UPLOAD_URL`|VCS upload url, required for enterprise github.||
 |`KUBECHECKS_WEBHOOK_SECRET`|Optional secret key for validating the source of incoming webhooks.||
 |`KUBECHECKS_WEBHOOK_URL_BASE`|The endpoint to listen on for incoming PR/MR event webhooks. For example, 'https://checker.mycompany.com'.||
 |`KUBECHECKS_WEBHOOK_URL_PREFIX`|If your application is running behind a proxy that uses path based routing, set this value to match the path prefix. For example, '/hello/world'.||

Feedback & Suggestions:

1. Clarification Needed: The description for KUBECHECKS_VCS_UPLOAD_URL mentions it's required for enterprise GitHub. It would be helpful to specify under what circumstances this is needed or provide an example to clarify its usage. 📝

2. Consistency Check: Ensure that the new entry follows the same format and style as the existing entries. This includes capitalization and punctuation consistency. For instance, consider whether "enterprise GitHub" should be capitalized as "Enterprise GitHub" to match the style of other entries. 🔍

3. Default Value: If applicable, consider specifying a default value for KUBECHECKS_VCS_UPLOAD_URL or explicitly state that there is no default. This helps users understand the expected configuration. ⚙️

---

---

Dependency Review

Click to read mergecats review!

No suggestions found