-
Notifications
You must be signed in to change notification settings - Fork 2
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
Traefik passthrough #29
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent updates refine domain configurations, enhance deployment scripts, and introduce new playbooks for key management. Notable changes include updated domain names for certificates, modified deployment arguments, and the addition of key management playbooks. Existing scripts were adjusted for consistency and improved functionality. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (6)
local_dev/config/pub/h1.pem
is excluded by!**/*.pem
local_dev/config/pub/h1_mk.pub
is excluded by!**/*.pub
local_dev/config/pub/h2.pem
is excluded by!**/*.pem
local_dev/config/pub/h2_mk.pub
is excluded by!**/*.pub
local_dev/config/pub/h3.pem
is excluded by!**/*.pem
local_dev/config/pub/h3_mk.pub
is excluded by!**/*.pub
Files selected for processing (18)
- .gitignore (1 hunks)
- README.md (1 hunks)
- ansible/deploy.yaml (1 hunks)
- ansible/inventory.ini (1 hunks)
- ansible/keygen.yaml (1 hunks)
- ansible/provision.yaml (1 hunks)
- ansible/upload_pub_keys.yaml (1 hunks)
- etc/start_helper_sidecar.sh (1 hunks)
- local_dev/config/h1.key (1 hunks)
- local_dev/config/h1_mk.key (1 hunks)
- local_dev/config/h2.key (1 hunks)
- local_dev/config/h2_mk.key (1 hunks)
- local_dev/config/h3.key (1 hunks)
- local_dev/config/h3_mk.key (1 hunks)
- local_dev/config/network.toml (4 hunks)
- sidecar/cli/cli.py (8 hunks)
- sidecar/traefik/dynamic-local/dynamic.yaml (1 hunks)
- sidecar/traefik/dynamic/dynamic.yaml (1 hunks)
Files skipped from review due to trivial changes (7)
- .gitignore
- local_dev/config/h1.key
- local_dev/config/h1_mk.key
- local_dev/config/h2.key
- local_dev/config/h2_mk.key
- local_dev/config/h3.key
- local_dev/config/h3_mk.key
Additional Context Used
LanguageTool (10)
README.md (10)
Near line 6: The plural noun “displays” cannot be used with the article “an”. Did you mean “an display” or “displays”?
Context: ...ont end and service that starts queries an displays logs from the MPC helper servers 2. dra...
Near line 7: “Sidecar” is a singular noun. It appears that the verb form is incorrect.
Context: ...ervers 2. draft-sidecar: a sidecar back end API that runs next to the IPA binary on...
Near line 16: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ng Locally Make sure the repo is cloned and you're working in the root directory of...
Near line 166: When ‘IPA-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...ivate pip install --editable . ``` ### IPA specific certs We check in self signed certs th...
Near line 168: This word is normally spelled with a hyphen.
Context: ...`` ### IPA specific certs We check in self signed certs that are only for local developme...
Near line 202: This word is normally spelled as one.
Context: ...ike to use. You will need to create two sub-domains,sidecar.example.com
and `helper.exam...
Near line 202: This word is normally spelled as one.
Context: ...ample.com. (Note, you could also use a sub-domain as your base domain, e.g.,
test.exampl...
Near line 202: This word is normally spelled as one.
Context: ...main, e.g.,test.example.com
with two sub-domains of that:sidecar.test.example.com
and...
Near line 219: Did you mean “once”?
Context: ...ryone's identity (e.g., 0, 1, 2, 3) One you know these: 1. Make a config direct...
Near line 228: When ‘IPA-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...le.com/privkey.pem key.pem5. Generate IPA specific keys: 1. Compile
ipawith
cargo ...
Additional comments not posted (19)
ansible/inventory.ini (1)
2-5
: The updated host entries withhelper_domain
andsidecar_domain
are correctly formatted and align with the PR objectives.Verification successful
The initial verification attempt failed due to a regex parse error. Let's correct the search pattern and re-verify the integration of
helper_domain
andsidecar_domain
in the Ansible files.
The updated host entries with
helper_domain
andsidecar_domain
are correctly formatted and align with the PR objectives. Additionally, the integration of these variables in other Ansible files has been verified.
helper_domain
is used in:
ansible/keygen.yaml
ansible/deploy.yaml
ansible/provision.yaml
sidecar_domain
is used in:
ansible/provision.yaml
ansible/deploy.yaml
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new domains are correctly integrated with other components. # Test: Search for the usage of `helper_domain` and `sidecar_domain` in other ansible files. rg --type yaml $'{{ helper_domain }}' ansible/ rg --type yaml $'{{ sidecar_domain }}' ansible/Length of output: 287
Script:
#!/bin/bash # Description: Verify that the new domains are correctly integrated with other components. # Test: Search for the usage of `helper_domain` and `sidecar_domain` in other ansible files. rg --type yaml 'helper_domain' ansible/ rg --type yaml 'sidecar_domain' ansible/Length of output: 741
etc/start_helper_sidecar.sh (1)
19-19
: Ensure the integration of the new parameters in thedraft run-helper-sidecar
command is tested thoroughly.sidecar/traefik/dynamic/dynamic.yaml (1)
16-30
: The configuration for TCP routers and services with TLS passthrough is correctly implemented. This setup is crucial for secure communication between services.ansible/upload_pub_keys.yaml (1)
22-25
: Ensure thenetwork.toml
file is correctly placed and accessible on the remote hosts.ansible/deploy.yaml (1)
27-28
: The inclusion ofhelper_domain
andsidecar_domain
in thestart-helper-sidecar
command aligns with the PR objectives. Ensure these domains are correctly resolved during deployment.sidecar/traefik/dynamic-local/dynamic.yaml (1)
55-101
: The configuration for TCP routers and services with TLS passthrough is correctly implemented for multiple helper domains. This setup is crucial for secure communication between services.ansible/keygen.yaml (7)
6-10
: This task correctly checks for the existence of the local directory and registers the result.
11-14
: The failure condition and message are clear and appropriately handled if the directory does not exist.
15-18
: The directory creation task is well-implemented with appropriate checks and uses Ansible's idempotent behavior.
24-30
: The task to clone and update the repository is correctly implemented using the Ansible 'git' module.
46-50
: The conditional build task is efficiently set up to run only if the binary does not exist, which saves unnecessary builds.
61-64
: The key generation command is well-constructed with all necessary parameters and is executed in the correct directory.
66-72
: The fetching of newly created files is correctly implemented with a loop to handle multiple files efficiently.local_dev/config/network.toml (3)
4-15
: The configuration for the first peer is correctly updated with the new domain names and certificate details.
24-35
: The configuration for the second peer is correctly updated with the new domain names and certificate details.
44-55
: The configuration for the third peer is correctly updated with the new domain names and certificate details.ansible/provision.yaml (1)
117-118
: The task to start the helper sidecar is correctly configured with the new domain parameters, ensuring proper setup.README.md (1)
173-175
: The commands for generating IPA-specific keys are correctly updated to reflect the new domain names and paths.sidecar/cli/cli.py (1)
77-93
: Thestart_traefik_command
function is correctly updated to handle the new domain parameters, with appropriate defaults and environment variable settings.
ansible/upload_pub_keys.yaml
Outdated
- hosts: all | ||
vars: | ||
local_public_key_directory: "{{ lookup('env', 'PWD') }}/.draft/deployed_public_keys" | ||
tasks: | ||
- name: Define remote_directory | ||
set_fact: | ||
remote_directory: "{{ ansible_env.HOME }}/draft/config" | ||
- name: Upload keys to remote hosts | ||
copy: | ||
src: "{{ local_public_key_directory }}/{{ item }}" | ||
dest: "{{ remote_directory }}/pub" | ||
loop: | ||
- "h0.pem" | ||
- "h0_mk.pub" | ||
- "h1.pem" | ||
- "h1_mk.pub" | ||
- "h2.pem" | ||
- "h2_mk.pub" | ||
- "h3.pem" | ||
- "h3_mk.pub" | ||
|
||
- name: Upload network.toml to remote hosts | ||
copy: | ||
src: "{{ local_public_key_directory }}/network.toml" | ||
dest: "{{ remote_directory }}/network.toml" |
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.
The playbook for uploading public keys is well-structured. Consider adding error handling to ensure the files exist before attempting to upload them.
+ - name: Check if public keys exist
+ stat:
+ path: "{{ item }}"
+ register: result
+ loop:
+ - "{{ local_public_key_directory }}/h0.pem"
+ - "{{ local_public_key_directory }}/h0_mk.pub"
+ - "{{ local_public_key_directory }}/h1.pem"
+ - "{{ local_public_key_directory }}/h1_mk.pub"
+ - "{{ local_public_key_directory }}/h2.pem"
+ - "{{ local_public_key_directory }}/h2_mk.pub"
+ - "{{ local_public_key_directory }}/h3.pem"
+ - "{{ local_public_key_directory }}/h3_mk.pub"
+ - "{{ local_public_key_directory }}/network.toml"
+ - name: Fail if any key does not exist
+ fail:
+ msg: "Key {{ item }} does not exist."
+ when: not result.stat.exists
+ loop: "{{ result.results }}"
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
- hosts: all | |
vars: | |
local_public_key_directory: "{{ lookup('env', 'PWD') }}/.draft/deployed_public_keys" | |
tasks: | |
- name: Define remote_directory | |
set_fact: | |
remote_directory: "{{ ansible_env.HOME }}/draft/config" | |
- name: Upload keys to remote hosts | |
copy: | |
src: "{{ local_public_key_directory }}/{{ item }}" | |
dest: "{{ remote_directory }}/pub" | |
loop: | |
- "h0.pem" | |
- "h0_mk.pub" | |
- "h1.pem" | |
- "h1_mk.pub" | |
- "h2.pem" | |
- "h2_mk.pub" | |
- "h3.pem" | |
- "h3_mk.pub" | |
- name: Upload network.toml to remote hosts | |
copy: | |
src: "{{ local_public_key_directory }}/network.toml" | |
dest: "{{ remote_directory }}/network.toml" | |
- hosts: all | |
vars: | |
local_public_key_directory: "{{ lookup('env', 'PWD') }}/.draft/deployed_public_keys" | |
tasks: | |
- name: Define remote_directory | |
set_fact: | |
remote_directory: "{{ ansible_env.HOME }}/draft/config" | |
- name: Check if public keys exist | |
stat: | |
path: "{{ item }}" | |
register: result | |
loop: | |
- "{{ local_public_key_directory }}/h0.pem" | |
- "{{ local_public_key_directory }}/h0_mk.pub" | |
- "{{ local_public_key_directory }}/h1.pem" | |
- "{{ local_public_key_directory }}/h1_mk.pub" | |
- "{{ local_public_key_directory }}/h2.pem" | |
- "{{ local_public_key_directory }}/h2_mk.pub" | |
- "{{ local_public_key_directory }}/h3.pem" | |
- "{{ local_public_key_directory }}/h3_mk.pub" | |
- "{{ local_public_key_directory }}/network.toml" | |
- name: Fail if any key does not exist | |
fail: | |
msg: "Key {{ item }} does not exist." | |
when: not result.stat.exists | |
loop: "{{ result.results }}" | |
- name: Upload keys to remote hosts | |
copy: | |
src: "{{ local_public_key_directory }}/{{ item }}" | |
dest: "{{ remote_directory }}/pub" | |
loop: | |
- "h0.pem" | |
- "h0_mk.pub" | |
- "h1.pem" | |
- "h1_mk.pub" | |
- "h2.pem" | |
- "h2_mk.pub" | |
- "h3.pem" | |
- "h3_mk.pub" | |
- name: Upload network.toml to remote hosts | |
copy: | |
src: "{{ local_public_key_directory }}/network.toml" | |
dest: "{{ remote_directory }}/network.toml" |
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ansible/upload_pub_keys.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- ansible/upload_pub_keys.yaml
helper<i>.draft.test
as a domain name for helpershelper<1>.ipa-helper.dev
as a domain name for helpersSummary by CodeRabbit
New Features
Improvements
Documentation