- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2.2k
 
Enable KAS #2598
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
Enable KAS #2598
Conversation
| 
           Are these settings all I need? I've tried to change on the running container but still doesn't work. On the clusters page I get the error message that GRPC is not available  | 
    
| 
           This seems to only set the configuration settings in case you're trying to use an external KAS install. The Omnibus repo seems to use some more steps to deploy the built-in KAS instance.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antt1995 Thank you for your contribution.
Those settings seems fine, but we need some step to make them accessible.
- 
Please add function (like
gitlab_configure_gitlab_kas()) toassets/runtime/functions
Maybe simplest reference isgitlab_configure_monitoring():
docker-gitlab/assets/runtime/functions
Lines 264 to 273 in b34d48a
gitlab_configure_monitoring() { echo "Configuring gitlab::monitoring..." update_template ${GITLAB_CONFIG} \ GITLAB_MONITORING_UNICORN_SAMPLER_INTERVAL \ GITLAB_MONITORING_IP_WHITELIST \ GITLAB_MONITORING_SIDEKIQ_EXPORTER_ENABLED \ GITLAB_MONITORING_SIDEKIQ_EXPORTER_ADDRESS \ GITLAB_MONITORING_SIDEKIQ_EXPORTER_PORT } 
So the implementation will be like:gitlab_configure_gitlab_kas() { echo "Configuring gitlab::KAS..." update_template ${GITLAB_CONFIG} \ GITLAB_KAS_ENABLED \ GITLAB_KAS_SECRET \ GITLAB_KAS_EXTERNAL \ GITLAB_KAS_INTERNAL \ GITLAB_KAS_PROXY }
 - 
Call the function in
configure_gitlab(). This function will be called on container startup.
docker-gitlab/assets/runtime/functions
Lines 1814 to 1877 in b34d48a
configure_gitlab() { echo "Configuring gitlab..." update_template ${GITLAB_CONFIG} \ GITLAB_INSTALL_DIR \ GITLAB_SHELL_INSTALL_DIR \ GITLAB_DATA_DIR \ GITLAB_REPOS_DIR \ GITLAB_DOWNLOADS_DIR \ GITLAB_SHARED_DIR \ GITLAB_HOME \ GITLAB_HOST \ GITLAB_PORT \ GITLAB_RELATIVE_URL_ROOT \ GITLAB_HTTPS \ GITLAB_SSH_HOST \ GITLAB_SSH_LISTEN_PORT \ GITLAB_SSH_PORT \ GITLAB_SIGNUP_ENABLED \ GITLAB_IMPERSONATION_ENABLED \ GITLAB_PROJECTS_LIMIT \ GITLAB_USERNAME_CHANGE \ GITLAB_DEFAULT_THEME \ GITLAB_CREATE_GROUP \ GITLAB_ISSUE_CLOSING_PATTERN gitlab_configure_database gitlab_configure_redis gitlab_configure_actioncable gitlab_configure_secrets gitlab_configure_sidekiq gitlab_configure_gitaly gitlab_configure_monitoring gitlab_configure_gitlab_workhorse gitlab_configure_relative_url gitlab_configure_trusted_proxies gitlab_configure_puma gitlab_configure_timezone gitlab_configure_rack_attack gitlab_configure_ci gitlab_configure_artifacts gitlab_configure_packages gitlab_configure_terraform_state gitlab_configure_lfs gitlab_configure_uploads gitlab_configure_mattermost gitlab_configure_project_features gitlab_configure_mail_delivery gitlab_configure_mailroom gitlab_configure_oauth gitlab_configure_ldap gitlab_configure_gravatar gitlab_configure_cron_jobs gitlab_configure_analytics gitlab_configure_backups generate_registry_certificates gitlab_configure_registry gitlab_configure_pages gitlab_configure_sentry generate_healthcheck_script gitlab_configure_content_security_policy # remove stale gitlab.socket rm -rf ${GITLAB_INSTALL_DIR}/tmp/sockets/gitlab.socket }  - 
(optional) The variable naming rule is not strictly defined, but it seems converting all yaml key name into UPPER_CASE is common approach for this repository.
- GITLAB_KAS_SECRET -> GITLAB_KAS_SECRET_FILE
 - GITLAB_KAS_EXTERNAL -> GITLAB_KAS_EXTERNAL_URL
 - GITLAB_KAS_INTERNAL -> GITLAB_KAS_INTERNAL_URL
 - GITLAB_KAS_PROXY -> GITLAB_KAS_K8S_PROXY_URL
 
 
| 
           @kkimurak  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passed build and test! Thanks for your contribution.
Sorry for forgetting to check in the last review, but could you please fill these additional tasks?
- add a description of the variables you added to the "Available Configuration Parameters" section of the README.md
 - check default values for the variables
 
| 
           Building Now with basic info in readme  | 
    
          
 Still not sure what to put but have put the vars and their defaults  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late, I have added review for README and fixed typo in my previous suggestion. Could you confirm them?
Best,
| 
           @kkimurak Confirmed Updated to Latest 15.1.2  | 
    
| 
           @antt1995 Do you mean you confirmed KAS works as expected applying this PR? Then I can approve. Thank you for your contribution and sorry for my rough review. By the way I have no permission to merge. Please ask for maintainer (  | 
    
          
 That's what I was Confirming  | 
    
| 
           @antt1995 ok Thank you. I approve the code changes themselves, but I'm not sure if KAS works.  | 
    
| 
           Would love this feature to be included  | 
    
| 
           Hi there, sorry for being late. I guess @kkimurak did a great job with his valuable hints. However, I don't use KAS and thus, I can't confirm that this PR works. Nevertheless, I'd like to ask if the configuration file for secrets (  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @antt1995 , Sorry for the tedious review process, but after being pointed out by @sachilles I reviewed the README again. Could you please confirm it?
edit: Oops sorry but I have added comment to "resolved" thread (and hidden by default). I apologize for the inconvenience, but for the second item, press "show resolved" and see the last comment.
| 
           Hello @antt1995 , I tested this pull request on my Gitlab instance but I haven't been able to make it work. The KAS was detected as installed by the UI as shown here: However, when adding a new agent, I always got the error message:  Should the service start by itself automatically by setting   | 
    
| 
           @eslamhossam23 Did you map the port in your docker config "-p 8153:8153" or in docker-compose "- 8153:8153"  | 
    
| 
           Hello @antt1995 yes, I had the port correctly mapped. The container itself haven't had started the KAS service inside it.  | 
    
| 
           @eslamhossam23 I deployed the KAS service with separate docker image and set the variables from this PR corresponding. Then it was working for me  | 
    
| 
           @eslamhossam23 As already reported above (#2598 (comment)), this image currently does not provides built-in KAS. This PR requires external KAS is exists. If you have time to do that, could you please try to build and test my  I have never used Kubernetes so never tested if the registration success, but made sure the  services:
  gitlab:
    environment:
      GITLAB_KAS_ENABLED: 'true'
      # If we don't set this, built-in gitlab-kas will exit with an error
      # See https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/8069d9b79763859b5bcead437a28f4dc73b66ae8/cmd/kas/kasapp/configured_app.go#L96-98
      OWN_PRIVATE_API_URL: grpc://127.0.0.1:8155 | 
    
76860a0    to
    452ae97      
    Compare
  
    | 
           Hi, is this PR considered to be ready to merge? I guess (and based on this discussion) there should be some comments on using KAS with this image (something like if you want using KAS you'll need to deploy an additional docker image), isn't it?  | 
    
          
 Is there any tutorial? @tiehfood  | 
    

Enabling KAS
Not sure what to put or where in the Readme