NOISSUE - Update cocos to match certs changes #520

WashingtonKK · 2025-09-14T15:04:43Z

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Related Issue #
Resolves #

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

codecov · 2025-09-14T15:17:45Z

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.81%. Comparing base (0be7243) to head (ce21e17).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/atls/certificate_provider.go	87.50%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   67.67%   67.81%   +0.14%     
==========================================
  Files          76       76              
  Lines        6833     6792      -41     
==========================================
- Hits         4624     4606      -18     
+ Misses       1860     1844      -16     
+ Partials      349      342       -7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

SammyOina · 2025-09-15T13:41:13Z

pkg/atls/atls.go

 			certsEndpoint := "certs"
 			csrEndpoint := "csrs"
-			endpoint := fmt.Sprintf("%s/%s/%s", certsEndpoint, csrEndpoint, cvmId)
+			endpoint := fmt.Sprintf("%s/%s/%s/%s", domainId, certsEndpoint, csrEndpoint, cvmId)


let's import and use sdk directly. Import sdk in main.go if caURL is provided and pass the sdk to internal or where needed. if not provided use the fallback. we don't need to duplicate sdk methods in am-certs

SammyOina · 2025-09-15T14:49:20Z

cmd/agent/main.go

+		CertsURL: cfg.CAUrl,
+		HostURL:  cfg.CAUrl,


this looks like it will result in an error if both are provided, or update certs sdk, since it only needs one url

SammyOina · 2025-09-15T14:50:44Z

cmd/agent/main.go

+		HostURL:  cfg.CAUrl,
+	}
+
+	crtSDK := certsSDK.NewSDK(ctrsdkconfig)


only init sdk if url is provided, otherwise agent should use default

SammyOina · 2025-09-15T14:51:12Z

cmd/agent/main.go

 	}

-	mc, err := cvmsapi.NewClient(pc, svc, eventsLogsQueue, logger, server.NewServer(logger, svc, cfg.AgentGrpcHost, cfg.CAUrl, cfg.CVMId), storageDir, reconnectFn, cvmGRPCClient)
+	mc, err := cvmsapi.NewClient(pc, svc, eventsLogsQueue, logger, server.NewServer(logger, svc, cfg.AgentGrpcHost, crtSDK, cfg.CAUrl, cfg.CVMId, cfg.DomainId), storageDir, reconnectFn, cvmGRPCClient)


update the function signature now that sdk is being passed remove redundant values

SammyOina · 2025-09-15T14:52:14Z

internal/server/grpc/grpc.go

 var _ server.Server = (*Server)(nil)

-func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, caUrl string, cvmId string) server.Server {
+func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, casdk sdk.SDK, caUrl, cvmId string, domainId string) server.Server {


remove values we no longer need

SammyOina · 2025-09-15T14:52:16Z

internal/server/grpc/grpc.go

 var _ server.Server = (*Server)(nil)

-func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, caUrl string, cvmId string) server.Server {
+func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, casdk sdk.SDK, caUrl, cvmId string, domainId string) server.Server {


remove values we no longer need

SammyOina · 2025-09-15T14:52:18Z

internal/server/grpc/grpc.go

 var _ server.Server = (*Server)(nil)

-func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, caUrl string, cvmId string) server.Server {
+func New(ctx context.Context, cancel context.CancelFunc, name string, config server.ServerConfiguration, registerService serviceRegister, logger *slog.Logger, authSvc auth.Authenticator, casdk sdk.SDK, caUrl, cvmId string, domainId string) server.Server {


remove values we no longer need

SammyOina · 2025-09-15T14:52:35Z

internal/server/grpc/grpc.go

 		tlsConfig := &tls.Config{
 			ClientAuth:     tls.NoClientCert,
-			GetCertificate: atls.GetCertificate(s.caUrl, s.cvmId),
+			GetCertificate: atls.GetCertificate(s.caSDK, s.caUrl, s.cvmId, s.domainId),


why are you passing caurl and sdk at the same time

SammyOina · 2025-09-15T14:53:25Z

pkg/atls/atls.go

-			url := fmt.Sprintf("%s/%s?%s", caUrl, endpoint, query_string)
-
-			_, body, err := processRequest(http.MethodPost, url, data, nil, http.StatusOK)
+			cert, err := caSDK.IssueFromCSR(cvmId, ttlString, string(csr.CSR), domainId, "")


which value is empty string

also shouldn't this be IssueFromCSRInternal absmach/certs#156 (comment)

SammyOina · 2025-09-16T13:31:57Z

pkg/atls/atls.go

-			url := fmt.Sprintf("%s/%s?%s", caUrl, endpoint, query_string)
-
-			_, body, err := processRequest(http.MethodPost, url, data, nil, http.StatusOK)
+			cert, err := caSDK.IssueFromCSRInternal(cvmId, ttlString, string(csr.CSR))


there should be a token/secret

This is yet to be added on certs.

SammyOina · 2025-09-16T13:32:59Z

test/cvms/main.go

+	caSDK := sdk.NewSDK(sdk.Config{
+		CertsURL: caUrl,
+	})


test cvm server does not use this, only agent, remove

dborovcanin · 2025-09-19T10:52:22Z

@WashingtonKK Please rebase.

dborovcanin · 2025-09-23T13:11:48Z

@WashingtonKK Please rebase.

drasko

LGTM

SammyOina · 2025-09-25T10:39:52Z

cmd/agent/main.go

+	LogLevel      string `env:"AGENT_LOG_LEVEL"   envDefault:"debug"`
+	Vmpl          int    `env:"AGENT_VMPL"        envDefault:"2"`
+	AgentGrpcHost string `env:"AGENT_GRPC_HOST"   envDefault:"0.0.0.0"`
+	CAUrl         string `env:"AGENT_CVM_CA_URL"  envDefault:""`
+	CVMId         string `env:"AGENT_CVM_ID"      envDefault:""`
+	DomainId      string `env:"AGENT_DOMAIN_ID"   envDefault:""`
+	CertsToken    string `env:"AGENT_CERTS_TOKEN" envDefault:""`
+	AgentMaaURL   string `env:"AGENT_MAA_URL"     envDefault:"https://sharedeus2.eus2.attest.azure.net"`
+	AgentOSBuild  string `env:"AGENT_OS_BUILD"    envDefault:"UVC"`
+	AgentOSDistro string `env:"AGENT_OS_DISTRO"   envDefault:"UVC"`
+	AgentOSType   string `env:"AGENT_OS_TYPE"     envDefault:"UVC"`


update agent service readme, also open a pr in cocos docs

SammyOina · 2025-09-25T10:40:50Z

manager/service.go

 	agentCvmClientKey       = "AGENT_CVM_GRPC_CLIENT_KEY"
 	agentCvmServerCaCertKey = "AGENT_CVM_GRPC_SERVER_CA_CERTS"
 	agentCvmId              = "AGENT_CVM_ID"
+	agentDomainId           = "AGENT_DOMAIN_ID"


what about ca token

also test to make sure this works from manager since this contant seems to be missing

SammyOina · 2025-09-25T10:41:39Z

manager/service.go

 		agentLogLevelKey:   req.AgentLogLevel,
 		agentCvmGrpcUrlKey: req.AgentCvmServerUrl,
 		agentCvmId:         id,
+		agentDomainId:      req.DomainId,


same here, test manager with agent

SammyOina · 2025-09-25T10:42:34Z

pkg/atls/atls.go

 type CSRRequest struct {
 	CSR string `json:"csr,omitempty"`
 }


import from certs

We do not have the CSRRequest on certs.

pkg/atls/atls.go

SammyOina · 2025-09-25T10:44:07Z

pkg/atls/atls.go

 }

-func (c *CAClient) processRequest(method, reqURL string, data []byte, headers map[string]string, expectedRespCodes ...int) (http.Header, []byte, errors.SDKError) {
+func (c *CAClient) processRequest(method, reqURL string, data []byte, headers map[string]string, token string, expectedRespCodes ...int) (http.Header, []byte, errors.SDKError) {


use sdk as stated in a previous comment we don't need to redifine this here

SammyOina · 2025-09-25T10:46:00Z

test/cvms/main.go

+	domainId          string
+	cvmId             string


these are not defined here, since you don't send them to agent and do not use them for anything

SammyOina · 2025-09-25T10:46:18Z

test/cvms/main.go

+	flagSet.StringVar(&cvmId, "cvm-id", "", "UUID for a CVM, must be specified if aTLS is used")
+	flagSet.StringVar(&domainId, "domain-id", "", "Domain ID for the attestation service, must be specified if aTLS is used")


SammyOina · 2025-09-25T10:47:08Z

pkg/atls/atls.go


-func (c *CAClient) RequestCertificate(csrMetadata certs.CSRMetadata, privateKey *ecdsa.PrivateKey, cvmID string, ttl time.Duration) ([]byte, error) {
+func (c *CAClient) RequestCertificate(csrMetadata certs.CSRMetadata, privateKey *ecdsa.PrivateKey, cvmID, domainId string, ttl time.Duration) ([]byte, error) {
 	csr, sdkerr := certscli.CreateCSR(csrMetadata, privateKey)


one of the first comments on this pr was to remove cli

dborovcanin · 2025-09-26T10:05:48Z

@WashingtonKK Please rebase.

SammyOina · 2025-09-26T12:41:29Z

agent/README.md

+| AGENT_CVM_GRPC_SERVER_KEY      | Path to gRPC server key in pem format                                                                         | ""                                              |
+| AGENT_CVM_GRPC_SERVER_CA_CERTS | Path to gRPC server CA certificate                                                                            | ""                                              |
+| AGENT_CVM_GRPC_CLIENT_CA_CERTS | Path to gRPC client CA certificate                                                                            | ""                                              |
+| AGENT_CVM_CA_URL               | URL for CA service, if provided it will be used for certificate generation, used only with aTLS at the moment | ""                                              |


domain id is missing

DomainId is not used.

Remove it from agent

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina · 2025-09-26T12:58:12Z

pkg/atls/atls_test.go

-		generator, err := NewProvider(nil, attestation.SNPvTPM, caURL, cvmID)
-		assert.NoError(t, err)
-		assert.NotNil(t, generator)
+		// Skip CA-signed test for now as it requires actual SDK implementation


use mocked sdk, do not skip tests

SammyOina

more comments

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina · 2025-09-29T15:05:10Z

cmd/agent/main.go

+	AgentGrpcHost string `env:"AGENT_GRPC_HOST"   envDefault:"0.0.0.0"`
+	CAUrl         string `env:"AGENT_CVM_CA_URL"  envDefault:""`
+	CVMId         string `env:"AGENT_CVM_ID"      envDefault:""`
+	DomainId      string `env:"AGENT_DOMAIN_ID"   envDefault:""`


this is unused

https://github.com/ultravioletrs/cocos/pull/520/files#r2382340551

SammyOina · 2025-09-29T15:13:32Z

cmd/agent/main.go

+	AgentGrpcHost string `env:"AGENT_GRPC_HOST"   envDefault:"0.0.0.0"`
+	CAUrl         string `env:"AGENT_CVM_CA_URL"  envDefault:""`
+	CVMId         string `env:"AGENT_CVM_ID"      envDefault:""`
+	DomainId      string `env:"AGENT_DOMAIN_ID"   envDefault:""`


https://github.com/ultravioletrs/cocos/pull/520/files#r2382340551

manager/manager.proto

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

pkg/atls/atls.go

pkg/atls/atls_test.go

pkg/atls/certificate_provider.go

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina requested changes Sep 15, 2025

View reviewed changes

WashingtonKK marked this pull request as ready for review September 16, 2025 12:38

SammyOina requested changes Sep 16, 2025

View reviewed changes

WashingtonKK changed the title ~~DRAFT - Update cocos to match certs changes~~ NOISSUE - Update cocos to match certs changes Sep 17, 2025

drasko approved these changes Sep 23, 2025

View reviewed changes

dborovcanin approved these changes Sep 23, 2025

View reviewed changes

WashingtonKK force-pushed the certs branch from 0ba0bd9 to 4bac65d Compare September 24, 2025 09:32

SammyOina requested changes Sep 25, 2025

View reviewed changes

WashingtonKK force-pushed the certs branch from 5a86a2f to e27df38 Compare September 26, 2025 10:11

SammyOina requested changes Sep 26, 2025

View reviewed changes

WashingtonKK added 13 commits September 26, 2025 15:56

pass domain id to agent environment

7be1a60

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

update generated files

21b9710

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

use certs sdk directly

0c1b1a2

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

remove redundant variables

43525b6

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

use agent certs token for csr

1f05b8a

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

update certs and add token to create req

e760b38

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

fix atls

ff71a18

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

add agent token to certificate provider

66f6f14

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

pass certs token to agent

e867cae

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

use sdk for csr

6b788fa

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

update atls

92778a6

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

fix tests

656b367

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

address comments

703a1e3

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

WashingtonKK force-pushed the certs branch from 9fcc453 to 703a1e3 Compare September 26, 2025 12:56

remove unused structs

0bc444d

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina requested changes Sep 26, 2025

View reviewed changes

WashingtonKK added 4 commits September 29, 2025 12:09

update tests

bbebb40

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

lint

aa1989b

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

fix tests

f369263

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

lint

068b9dc

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina requested changes Sep 29, 2025

View reviewed changes

manager/manager.proto Outdated Show resolved Hide resolved

SammyOina requested changes Sep 29, 2025

View reviewed changes

manager/manager.proto Outdated Show resolved Hide resolved

WashingtonKK added 2 commits September 29, 2025 18:27

remove unused domain id

4204d33

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

refactor tests and remove unused struct fields

39cbec5

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina requested changes Sep 29, 2025

View reviewed changes

pkg/atls/atls.go Outdated Show resolved Hide resolved

pkg/atls/atls_test.go Outdated Show resolved Hide resolved

pkg/atls/atls_test.go Outdated Show resolved Hide resolved

pkg/atls/certificate_provider.go Outdated Show resolved Hide resolved

WashingtonKK added 3 commits September 30, 2025 00:36

refactor(atls): remove CAClient and inline CA certificate issuance

da68ce0

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

lint'

b0ece26

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

increase coverage

032628d

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina approved these changes Sep 30, 2025

View reviewed changes

WashingtonKK added 3 commits October 6, 2025 11:35

fix bug in certs sdk and certificate provider

4303f26

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

update certs

df3ff58

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

fix pkg stress

ce21e17

Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

SammyOina approved these changes Oct 6, 2025

View reviewed changes

SammyOina assigned WashingtonKK Oct 6, 2025

drasko merged commit 0ffc2d1 into ultravioletrs:main Oct 6, 2025
11 checks passed

		flagSet.StringVar(&cvmId, "cvm-id", "", "UUID for a CVM, must be specified if aTLS is used")
		flagSet.StringVar(&domainId, "domain-id", "", "Domain ID for the attestation service, must be specified if aTLS is used")

NOISSUE - Update cocos to match certs changes #520

NOISSUE - Update cocos to match certs changes #520

Uh oh!

Conversation

WashingtonKK commented Sep 14, 2025

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

Uh oh!

codecov bot commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dborovcanin commented Sep 19, 2025

Uh oh!

dborovcanin commented Sep 23, 2025

Uh oh!

drasko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dborovcanin commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov bot commented Sep 14, 2025 •

edited

Loading

dborovcanin commented Sep 26, 2025 •

edited

Loading