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

Add OpAMP connection settings capability to the example #174

Conversation

tigrannajaryan
Copy link
Member

This change demonstrates OpAMP Connection Setting Offer Flow from OpAMP spec (see https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#opamp-connection-setting-offer-flow)

  • The example agent and server now use TLS for OpAMP connection.
  • The server's certificate is signed by a CA authority. The CA authority's certificate is self-generated (can be re-generated using examples/certs/generate.sh).
  • The server may generate and offer a client-side certificate to the agent. The generated certificate is signed by the same CA.
  • The client can accept the certificate and use it for future connections.
  • Server UI now allows the user to generate a certificate to offer to the agent (for the first time or to rotate). The flows are described in the OpAMP spec, see https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#trust-on-first-use

WARNING: all included certificate files are examples only and MUST NOT
be used in production.

How to test:

  1. Run the server.
  2. Run the agent.
  3. Go to http://localhost:4321/
  4. Find the connected agent in the list and click it
  5. Scroll down and click "Accept and Offer Client Certificate" button.
    image
  6. The certificate should be generated, sent to agent, accepted and agent will reconnect and the details should be shown:
    image

@tigrannajaryan tigrannajaryan requested a review from a team June 21, 2023 19:59
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 88.61% and project coverage change: -0.51 ⚠️

Comparison is base (efddaa2) 76.11% compared to head (c45862c) 75.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #174      +/-   ##
==========================================
- Coverage   76.11%   75.61%   -0.51%     
==========================================
  Files          24       24              
  Lines        1834     1923      +89     
==========================================
+ Hits         1396     1454      +58     
- Misses        326      352      +26     
- Partials      112      117       +5     
Impacted Files Coverage Δ
client/internal/receivedprocessor.go 69.28% <ø> (-15.00%) ⬇️
client/internal/wsreceiver.go 91.42% <ø> (ø)
client/types/callbacks.go 66.66% <ø> (ø)
server/httpconnection.go 33.33% <0.00%> (ø)
client/internal/mockserver.go 82.35% <85.71%> (+0.31%) ⬆️
client/internal/clientcommon.go 81.22% <87.50%> (+1.42%) ⬆️
server/serverimpl.go 58.92% <94.73%> (+0.37%) ⬆️
client/httpclient.go 95.00% <100.00%> (+0.26%) ⬆️
client/internal/httpsender.go 72.22% <100.00%> (+1.25%) ⬆️
client/wsclient.go 88.95% <100.00%> (+4.20%) ⬆️
... and 2 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

internal/examples/agent/agent/agent.go Outdated Show resolved Hide resolved
internal/examples/agent/agent/agent.go Outdated Show resolved Hide resolved
internal/examples/agent/agent/agent.go Show resolved Hide resolved
internal/examples/agent/agent/agent.go Show resolved Hide resolved
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/democonnsettings branch from 54300ea to f63265c Compare June 26, 2023 17:07
Copy link
Member Author

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

@srikanthccv thanks for reviewing.

internal/examples/agent/agent/agent.go Show resolved Hide resolved
internal/examples/agent/agent/agent.go Outdated Show resolved Hide resolved
internal/examples/agent/agent/agent.go Outdated Show resolved Hide resolved
internal/examples/agent/agent/agent.go Show resolved Hide resolved
This change demonstrates OpAMP Connection Setting Offer Flow
from OpAMP spec (see https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#opamp-connection-setting-offer-flow)

- The example agent and server now use TLS for OpAMP connection.
- The server's certificate is signed by a CA authority. The CA
  authority's certificate is self-generated (can be re-generated
  using examples/certs/generate.sh).
- The server may generate and offer a client-side certificate to the agent.
  The generated certificate is signed by the same CA.
- The client can accept the certificate and use it for future
  connections.
- Server UI now allows the user to generate a certificate to
  offer to the agent (for the first time or to rotate). The flows
  are described in the OpAMP spec, see
  https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#trust-on-first-use

 WARNING: all included certificate files are examples only and MUST NOT
 be used in production.

 How to test:
 1. Run the server.
 2. Run the agent.
 3. Go to http://localhost:4321/
 4. Find the connected agent in the list and click it
 5. Scroll down and click "Accept and Offer Client Certificate" button.
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/democonnsettings branch from f63265c to c45862c Compare June 26, 2023 17:09
Copy link
Member

@srikanthccv srikanthccv 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 addressing comments

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@tigrannajaryan
Copy link
Member Author

@andykellr OK to merge this or you would like to review?

@tigrannajaryan tigrannajaryan merged commit 13a6fd1 into open-telemetry:main Jun 30, 2023
@tigrannajaryan tigrannajaryan deleted the feature/tigran/democonnsettings branch June 30, 2023 03:54
tigrannajaryan added a commit to tigrannajaryan/opamp-go that referenced this pull request Jul 6, 2023
open-telemetry#174 enabled
TLS on the server and non-TLS connections are no longer
accepted.

This PR updates the Supervisor to use TLS connection
(with InsecureSkipVerify=true flag).
tigrannajaryan added a commit that referenced this pull request Jul 6, 2023
#174 enabled
TLS on the server and non-TLS connections are no longer
accepted.

This PR updates the Supervisor to use TLS connection
(with InsecureSkipVerify=true flag).
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