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

Native deployment for Veraison #250

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Native deployment for Veraison #250

merged 6 commits into from
Aug 13, 2024

Conversation

setrofim
Copy link
Collaborator

@setrofim setrofim commented Aug 8, 2024

This pull adds a "native" deployment for Veraison. This is similar to the existing "docker" deployment, but does not create docker containers, and instead runs natively on the system. The downside is that it has non-trivial dependencies (the deployment features boostrap scripts for Arch, Ubuntu, and MacOSX). The upside is that it is significantly faster to create, easier to use for development and debugging, and does not rely on Docker being set up on the system.

Ensure that $(go env GPATH)/bin is in PATH before running protoc, as it
looks for plugins (such as protoc-gen-go) there.

Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Keycloak's logging lib expects flags to be parsed, otherwise it
complains, so we're parsing them. The problem is that if we actually
have any flags specified, parsing will fail, as the lib doesn't know
about Veraison flags, so clear os.Args of flags before calling
flags.Parse().

This is fine as KeycloakAuthorizer.Init(), where the call occurs, is
invoked well after the service command line has already been processed,
so os.Args won't be ever be used from then on.

This does mean that we can no longer pass configuration for Keycloak
logging lib on the command line, which was theoretically possible
before. However we've literally never do this, it's completely
undocumented, and may potentially cause conflicts with Veraison's own
logging. So this fixes that as well.

Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Add "ca-cert" config for "keycloak" auth backend. This allows specifying
and additional CA cert to be used when connecting to the Keycloak
authentication server. This removes the need for the cert to installed
in the system.

Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

a handful of small fixes inlined

end-to-end/end-to-end-native Outdated Show resolved Hide resolved
deployments/native/bin/veraison Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

thanks!

Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

Thanks for this work!

In general LGTM, I will re-try the setup on my local machine to check, everything is working as expected! The intention is to see any further improvements are needed or not.

Between, left some nits and questions for you to clarify in this review!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
deployments/native/README.md Outdated Show resolved Hide resolved
deployments/native/deployment.sh Outdated Show resolved Hide resolved
deployments/native/deployment.sh Outdated Show resolved Hide resolved
Add scripts to build, deploy, and run Veraison natively on the host
system.

While this is primarily a "local" deployment, i.e. intended to be run
locally for development/testing/demo purposes (similar to the existing
docker deployment), this is also intended to provide a basis for future
production deployments as well.

- Boostrap scripts are provided to ensure suitable build environment.
- Deployment steps can be executed separately, to e.g. omit client
  installation.
- Commands for generating certificates and the signing key are included
  in the frontend (though the use of pre-created "example" certs and
  keys is still possible).
- While the quick deployment is user-specific, system-level systemd
  units are also included, so its possible to, e.g. deploy into /opt
  and then install system units to provide a deployment that runs
  independent of user login. (note: a full system deployment would
  require dedicated user configuration, symlinking executables into
  /usr/local/bin, etc... That currently doesn't exist.)

In addition to the deployment code itself, this commit makes the
following amendments:

- end-to-end flow is updated to work with the native deployment, as well
  as docker via an alternate script, end-to-end-native. The existing
  script is renamed to end-to-end-docker.
- The top-level README.md is updated with native deployment
  instructions.
- Fixed line breaks inside top-level README.md to conform to the 79
  column standard.
- Fixed admonitions inside top-level REAME.md to use the GitHub extended
  syntax.

Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

couple of small edits

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>
Co-authored-by: Sergei Trofimov <sergei.trofimov@arm.com>
@setrofim setrofim force-pushed the native branch 2 times, most recently from 6709120 to a57ae54 Compare August 13, 2024 13:03
Signed-off-by: Thomas Fossati <thomas.fossati@linaro.org>
Co-authored-by: Sergei Trofimov <sergei.trofimov@arm.com>
@yogeshbdeshpande
Copy link
Collaborator

yogeshbdeshpande commented Aug 13, 2024

@setrofim : Thanks for enabling the support for WSL users. We have validated the setup on my machine and now it all works to completion!

@setrofim setrofim merged commit b057e3f into main Aug 13, 2024
8 checks passed
@setrofim setrofim deleted the native branch August 13, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants