Skip to content
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

fix(agw): fix certificate issue for containerized AGW deployment #13862

Conversation

wolfseb
Copy link
Contributor

@wolfseb wolfseb commented Sep 7, 2022

Signed-off-by: Sebastian Wolf sebastian.wolf@tngtech.com

Summary

Resolves issue #13682.
This PR fixes the issue where a containerized deployment on bare metal tries to mount the rootCA.pem certificate from the directory$MAGMA_ROOT/.cache/test_certs/, which does not exist. Instead, it will be mounted from /var/opt/magma/certs/, where it is previously copied to when following the documentation.
This should not affect the containerized AGW setup inside the dev VM, as spinning up the VM creates the certificate in both locations: at $MAGMA_ROOT/.cache/test_certs/ (mounted from host) and at /var/opt/magma/certs/ in the VM.

Test Plan

  • test that the containerized AGW deployment in the dev environment (i.e. inside the dev VM) is unaffected by this change
  • test the error in the linked issue for a deployment of the AGW containers on bare metal is resolved

Additional Information

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines. label Sep 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added the component: ci All updates on CI (Jenkins/CircleCi/Github Action) label Sep 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

feg-workflow

    2 files  203 suites   40s ⏱️
374 tests 374 ✔️ 0 💤 0
388 runs  388 ✔️ 0 💤 0

Results for commit 6546140.

♻️ This comment has been updated with latest results.

@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from a4fc463 to 7d38c7c Compare September 7, 2022 09:16
@wolfseb wolfseb marked this pull request as ready for review September 7, 2022 09:40
@wolfseb wolfseb requested a review from a team September 7, 2022 09:40
@wolfseb wolfseb requested a review from a team as a code owner September 7, 2022 09:40
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

dp-workflow

14 tests   14 ✔️  3m 7s ⏱️
  1 suites    0 💤
  1 files      0

Results for commit 6546140.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

agw-workflow

615 tests   611 ✔️  3m 48s ⏱️
    2 suites      4 💤
    2 files        0

Results for commit 6546140.

♻️ This comment has been updated with latest results.

@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch 3 times, most recently from fceb588 to 25e7135 Compare September 13, 2022 22:56
@wolfseb wolfseb added DONOTLAND Please don't merge this and removed DONOTLAND Please don't merge this labels Sep 20, 2022
@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from e99ef11 to c38e530 Compare September 21, 2022 00:54
@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from c38e530 to dd3cfd4 Compare September 21, 2022 01:05
@@ -47,6 +47,13 @@ docker-compose build
docker-compose up
```

Optional: If you want to connect to an orc8r, copy the `rootCA.pem` from the orc8r
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should happen before we run docker-compose up for the first time, so we should move this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we mount the directory anyway I would have assumed that the order doesn't really matter, but I can change it if that is better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the AGW will read the certificate on startup and probably not re-read it when it is copied later when the AGW is already running.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved it before the docker setup.

@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from dd3cfd4 to 3911d1e Compare September 21, 2022 07:31
@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from 3911d1e to a4336b7 Compare September 21, 2022 08:03
@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from a4336b7 to 08f87bf Compare September 21, 2022 09:28
@mpfirrmann mpfirrmann added the ready2merge This PR is ready to be merged (is approved and passes all required checks) label Sep 21, 2022
fixes the mount path for the rootCA.pem certificate, so a containerized deployment
outside the dev VM can still access it

Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
@wolfseb wolfseb force-pushed the fix_certificate_issue_for_containerized_AGW_deployment branch from 08f87bf to 6546140 Compare September 25, 2022 23:18
@voisey voisey merged commit e736de2 into magma:master Sep 28, 2022
pruthvihebbani pushed a commit to pruthvihebbani/magma that referenced this pull request Oct 10, 2022
…ma#13862)

* fix(agw): fix certificate issue for containerized AGW deployment

fixes the mount path for the rootCA.pem certificate, so a containerized deployment
outside the dev VM can still access it

Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>

* chore: add option to update rootCA.pem in deployment docs

Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>

Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ci All updates on CI (Jenkins/CircleCi/Github Action) ready2merge This PR is ready to be merged (is approved and passes all required checks) size/XS Denotes a PR that changes 0-9 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker-compose build creates rootCA.pem directory instead of copying it as file
5 participants