-
-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GitLab Comment Reporter always reports a certificate error talking to GitLab #1239
Comments
This looks similar to this issue: #1125 (comment) Please could you adapt and try these lines in your .mega-linter.yml ?
|
I adapted and added the commands suggested. These essentially duplicate my pre_script in the GitLab pipeline. I get the following output from the pre_script:
Output from PRE_COMMANDS:
I still get the error:
I added a POST_COMMAND for curl to check access and validate certificate:
curl seems happy. Since the POST_COMMANDS run before the GITLAB_COMMENT_REPORTER, I also added an "after_script" to the CI Pipeline to check again. This gives the same results as before, working OK.
It just seems Python is refusing to use this when GITLAB_COMMENT_REPORTER is running. |
I put in a few extra debug statements in my GitLab CI before_script.
If I point directly at the /etc/ssl/cert.pem file, it is OK. Note that it is in the list of ssl.get_default_verify_paths() output. If I remove the verify parameter from the request call, it throws the error again:
Almost seems like a bug in python/urlllib3/ssl. OpenSSL Version: OpenSSL 1.1.1l 24 Aug 2021 |
Lastly, for now anyway, I also did some directory listings. /etc/ssl/cert.pem is a Symbolic Link to /etc/ssl/certs/ca-certificates.crt so I did a basic python request with /etc/ss/certs/ca-certificates.crt as the verify parameter:
That works too. Why is python not properly using these locations for verification with it's defaults pointing to the same location? |
The library used to interact with gitlab is python-gitlab https://python-gitlab.readthedocs.io/en/stable/ , maybe it needs to be called with some additional parameters, i'll try to have a look (you can too, you seem very good at investigaing ^^ ) |
The odd thing, though, is that regular python has issues unless we supply the verify parameter, and even though I have provided the path to a file it should already be using, as per the default_verify_paths, it only works when the path is supplied explicitly. The testing I have done is using the following, default, paths:
Both are in Python's default paths for ssl verification, and are updated by the method you provided. Providing either path to python-gitlab explicitly should work for anyone adding a custom certificate per this manner. |
@jberkers42 I think I'm close to a generic solution that would avoid to use pre_commands
So with current solution, you still need to use the following pre_commands : - command: curl -L -o /usr/local/share/ca-certificates/RootCA.crt http://path.to.cert
cwd: root
- command: curl -L -o /usr/local/share/ca-certificates/IssueCA.crt http://path.to.cert
cwd: root What if I added something like: Thanks ! |
While I think this would work, I'm not 100% sure this is the right way. We have our runners in a Kubernetes environment, and the right way there is to provide a "Secret" that is subsequently mounted as a file under /etc/ssl/certs. That would be the _right_way for kubernetes. When operating with Docker runners there would be a similar process where the runner instructs Docker to mount the certificate file at the same location. This method should scale for all other scenarios that require an internal CA or other custom certificate bundle to be provided. We already do this for the runner-helper that runs in Kubernetes, for it to properly validate the GitLab certificate. I think the use of the GITLAB_CERTIFICATE_PATH possibly the best way to ensure ssl_verify is performed against the right set of certificates. At the same time I understand that this may not work for everyone, so having a method to supply a certificate by other means is helpful. My initial method used a GitLab variable to simply pass the contents of the certificate file along to be written out by the GitLab pre_script. I did find that newlines were replaced with spaces in the variable, so had to do something to correct for that (I used sed). I had to hunt for a little bit to find somewhere to host the certificate file on a cleartext web service, so that may be a challenge for others also, in addition to adding a dependency to the environment. Can you do conditional? If GITLAB_CERTIFICATE_PATH is provided, use that, if GITLAB_CUSTOM_CERTIFICATE is provided, write contents to a file, and use this file in ssl_verify. What do you think? Let me know if I've been unclear on various points. |
MegaLinter can do whatever it wants with ifs :) # Build gitlab options
gitlab_options = {}
# auth token
if config.get("GITLAB_ACCESS_TOKEN_MEGALINTER", "") != "":
gitlab_options.private_token = config.get(
"GITLAB_ACCESS_TOKEN_MEGALINTER"
)
else:
gitlab_options.job_token = config.get("CI_JOB_TOKEN")
# certificate management
if config.get("GITLAB_CERTIFICATE_PATH", "") != "":
run_command(
{"cwd": "root", "command": "update-ca-certificates"},
"GitlabCommentReporter",
self.master,
)
gitlab_options.ssl_verify = config.get("GITLAB_CERTIFICATE_PATH")
# Create gitlab connection
gl = gitlab.Gitlab(gitlab_server_url, gitlab_options) Can i ask you to complete it with pseudo-code, then I'll adapt it ? |
My attempt at psuedo-code for this: # Build gitlab options
gitlab_options = {}
# auth token
if config.get("GITLAB_ACCESS_TOKEN_MEGALINTER", "") != "":
gitlab_options.private_token = config.get(
"GITLAB_ACCESS_TOKEN_MEGALINTER"
)
else:
gitlab_options.job_token = config.get("CI_JOB_TOKEN")
# certificate management
if config.get("GITLAB_CERTIFICATE_PATH", "") != "":
run_command(
{"cwd": "root", "command": "update-ca-certificates"},
"GitlabCommentReporter",
self.master,
)
gitlab_options.ssl_verify = config.get("GITLAB_CERTIFICATE_PATH")
elseif config.get("GITLAB_CUSTOM_CERTIFICATE", "") != "":
write contents to /etc/ssl/certs/gitlab-cert.crt
gitlab_options.ssl_verify = "/etc/ssl/certs/gitlab-cert.crt"
# Create gitlab connection
gl = gitlab.Gitlab(gitlab_server_url, gitlab_options) Hope that makes sense. The command I had used to write the certificate from variable was:
Since the line-breaks in the certificate contents seem to get replaced with spaces, I had to put them back. I uses a '#' in the header as a temporary substitute so that it didn't get split. |
@jberkers42 thanks for the detailed info :) I updated the code this way, and also the reporter documentation # Certificate management
gitlab_certificate_path = config.get("GITLAB_CERTIFICATE_PATH", "")
if config.get("GITLAB_CUSTOM_CERTIFICATE", "") != "":
# Certificate value defined in an ENV variable
cert_value = config.get("GITLAB_CUSTOM_CERTIFICATE")
gitlab_certificate_path = "/etc/ssl/certs/gitlab-cert.crt"
with open(gitlab_certificate_path, "w", encoding="utf-8") as cert_file:
cert_file.write(cert_value)
logging.debug(
f"Updated {gitlab_certificate_path} with certificate value {cert_value}"
)
if gitlab_certificate_path != "":
# Update certificates and set cert path in gitlab options
run_command(
{"cwd": "root", "command": "update-ca-certificates"},
"GitlabCommentReporter",
self.master,
)
gitlab_options.ssl_verify = gitlab_certificate_path
# Create gitlab connection
logging.debug(
f"[GitlabCommentReporter] Logging to {gitlab_server_url} with {str(gitlab_options)}"
)
gl = gitlab.Gitlab(gitlab_server_url, gitlab_options) Please can I ask you to test on your side with docker image |
I set the GITLAB_CUSTOM_CERTIFICATE variable, and tried it:
|
basic python mistake... oops :) Please can you try again with updated megalinter/megalinter:test-nvuillam-gitlab-cert ? |
Works much better now. Thank you once again for your efforts. I will probably do some more extensive tests tomorrow just to be sure, it's nearly midnight here. |
That was a shared effort, thanks for helping MegaLinter being better :) So without PRE_COMMANDS and with just GITLAB_ACCESS_TOKEN_MEGALINTER and GITLAB_CUSTOM_CERTIFICATE we're all good ? I think we can't do smaller custom config, that's great ^^ 14h here ... I understand, good night :) I'll probably have published in beta tomorrow, at worse if there are issues i'll make a new PR :) |
Just a quick update. I have performed various tests, and am happy to report success on both modes of the functionality:
Both work as advertised without any issues. In my continued testing, I did encounter some issues with markdownlinkcheck that were related to certificates. I am currently using a GitLab CI before_script to do update-ca-certificates to fix this after providing the certificate bundle via Kubernetes. I made the following change to my gitlab-runner.yml helm chart (for other's reference): runners:
config:
[[runners]]
name = "name-goes-here"
executor = "kubernetes"
[runners.kubernetes]]
[[runners.kubernetes.volumes.secret]]
name = "customca"
mount_path = "/usr/local/share/ca-certificates"
read_only = true
[runners.kubernetes.volumes.secret.items]
"custom_ca.crt" = "custom_ca.crt" To make use of the above, I have registered a secret with the same name in the namespace of the gitlab-runner containing the certificate. kubectl create secret generic customca --namespace gitlabci --from-file=custom_ca.crt Thanks again for your prompt attention and assistance with this. |
Thanks for your tests and feedback ! Do you have internally hosted URLs that are tested by markdown-link-check ? Note: you appear in the doc now haha ^^ -> https://megalinter.github.io/beta/reporters/GitlabCommentReporter/#special-thanks |
Yes, indeed. In many of the CHANGELOG.md documents in various projects I have reference links to the repo, issues and releases, that all point back to out internal gitlab instance. Needless to say, if the certificate is not trusted, the link tests fail, but don't really tell me why they fail. When trying to fix the cert for the comment reporter, the link checks stopped failing. |
markdown-link-check uses needle behind the scenes, and it seems it gets local system certificate info to perform the calls, that could explain why it works after your updates :) |
Describe the bug
Since version 5.7 the GitLab Comment Reporter always throws a certificate error as follows:
We currently host our GitLab instance on an Azure VM, with a Kubernetes GitLab Runner in AKS with Spot Instances on GitLab 14.7. The SSL Certificate is signed by our internal CA, and the Kubernetes Runners get the certificate as part of the Runner configuration.
I have further ensured that the CA Certificate is in the list of trusted certificates on the runner using pre_script steps in our GitLab CI pipeline to:
We configured the required settings for GITLAB_COMMENT_REPORTER to be able to work, such as the Personal Access Token and the Server and API URL values as per documentation guidance.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Expect GitLab API/Server connection to succeed, perhaps encounter an Authentication error.
Screenshots
Variables configured:
Additional context
Prior to ensuring the CA Certificate was present using pre_script steps, the markdown-link-checker was also failing on references to our GitLab instance. These errors have since gone away, however, the GITLAB_COMMENT_REPORTER issues persist.
Let me know if you need additional information.
The text was updated successfully, but these errors were encountered: