Skip to content
This repository was archived by the owner on Apr 6, 2026. It is now read-only.

Certs - Remove all magistrala dependencies#4

Merged
dborovcanin merged 17 commits into
absmach:mainfrom
nyagamunene:certs-1
Aug 27, 2024
Merged

Certs - Remove all magistrala dependencies#4
dborovcanin merged 17 commits into
absmach:mainfrom
nyagamunene:certs-1

Conversation

@nyagamunene

Copy link
Copy Markdown
Contributor

What type of PR is this?

This is a refactor because it changes the following functionality because it removes Magistrala dependencies to avoid cyclic imports.

What does this do?

It removes Magistrala dependencies and implements some of them in the repo.

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

Have you included tests for your changes?

Yes.

Did you document any new/modified features?

No.

Notes

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene self-assigned this Aug 19, 2024
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

@arvindh123 arvindh123 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@dborovcanin , How this certs will be working?
Like it will be replacing only vault or it will be replacing entire certs service in Magistrala?

Comment thread service.go Outdated
return "", err
}

func (s *service) IssueCert(ctx context.Context, userId, entityID string, entityType EntityType, ipAddrs []string) (string, error) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it will be good, if we provide all the options Organization, Orgnaization Unit,CN, Subject Alternative Names, etc...
@dborovcanin
Please give your comments

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree all options should be available on issuing

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene marked this pull request as ready for review August 21, 2024 11:21

@SammyOina SammyOina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

import common utilities from mg to avoid code duplication

Comment thread .gitignore Outdated
Comment thread Makefile Outdated
Comment thread api/grpc/server.go
return &grpcServer{
getEntity: kitgrpc.NewServer(
otelkit.EndpointMiddleware(otelkit.WithOperation("get_entity"))(getEntityEndpoint(svc)),
(getEntityEndpoint(svc)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why have you removed tracing

Comment thread api/grpc/server.go
return &grpcServer{
getEntity: kitgrpc.NewServer(
otelkit.EndpointMiddleware(otelkit.WithOperation("get_entity"))(getEntityEndpoint(svc)),
(getEntityEndpoint(svc)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why have you removed tracing

Comment thread api/http/responses.go Outdated
Comment thread docker/docker-compose.yml Outdated
ports:
- ${AM_JAEGER_FRONTEND}:${AM_JAEGER_FRONTEND}
- ${AM_JAEGER_OLTP_HTTP_PORT}:${AM_JAEGER_OLTP_HTTP_PORT}

No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change

Comment thread docker/ssl/ca.crt Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a standalone serevice, root ca should be generated by the service itself during initialization

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you import form mg to avoid repetition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Comment thread pkg/apiutil/errors.go Outdated
@@ -0,0 +1,47 @@
// Copyright (c) Abstract Machines

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is a single service, we do not need apiutil package from monorepo, and probably there's no need for some of these errors. Remove this package and add errors that make sense to API layer (simple errors.go file will work).

Comment thread pkg/apiutil/transport.go Outdated
@@ -0,0 +1,123 @@
// Copyright (c) Abstract Machines

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same, everything from pkg/apiutil belongs to api directly. Check if all of these methods are needed.

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>

@dborovcanin dborovcanin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fix the Dockerfile by removing web leftovers:

FROM golang:1.23-alpine AS builder

ARG SVC
ARG GOARCH
ARG GOARM
ARG VERSION
ARG COMMIT
ARG TIME

WORKDIR /go/src/github.com/absmach/certs

COPY . .

RUN apk update \
    && apk add make \
    && make $SVC \
    && mv build/$SVC /exe

FROM scratch

# Rrequired for billing service
COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
COPY --from=builder /exe /
ENTRYPOINT ["/exe"]

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@dborovcanin dborovcanin merged commit 1bea63e into absmach:main Aug 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Remove Auth from Certs repository

4 participants