Skip to content

feat: add base key api and announce key req#132

Open
NamanBalaji wants to merge 2 commits into
mainfrom
feat/key-model-and-announce-key-req
Open

feat: add base key api and announce key req#132
NamanBalaji wants to merge 2 commits into
mainfrom
feat/key-model-and-announce-key-req

Conversation

@NamanBalaji
Copy link
Copy Markdown
Contributor

No description provided.

@NamanBalaji NamanBalaji force-pushed the feat/key-model-and-announce-key-req branch from bbdce53 to 728d7dd Compare May 8, 2026 18:08
@NamanBalaji NamanBalaji self-assigned this May 11, 2026
Comment thread api-specs/v1/proto/keys/keys.proto Outdated
syntax = "proto3";


package krypton.v1.keys;
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 was wondering if we don't want to keep the packages organized by consumer type meaning moving the KeyService to the krypton.v1.admin package. This would also mean we need to prefix the existing Service with Tenant. This would group admin-related operations together and make the API more intuitive for consumers

Comment thread pkg/api/v1/proto/admin/admin_grpc.pb.go Outdated
// Code generated by protoc-gen-go-grpc. DO NOT EDIT.
// versions:
// - protoc-gen-go-grpc v1.6.1
// - protoc-gen-go-grpc v1.5.1
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.

We should use the latest version if possible

Comment thread pkg/model/key.go
"github.com/openkcm/krypton/internal/clock"
)

type KeyState string
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 think the intention was to have a processing state like this state and a key (lifecycle) state which is already defined by the keylifecycle.State, so I'd rename this to be more precise

Copy link
Copy Markdown
Contributor

@jithinkunjachan jithinkunjachan left a comment

Choose a reason for hiding this comment

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

Nicely done

Comment thread pkg/store/sql/key.go Outdated
return nil, err
}

key.Kind = kind
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.

Do we require this shall we put it directly inside the row.Scan ?

Suggested change
key.Kind = kind

"github.com/openkcm/krypton/internal/config"
)

const DefaultMaxPendingReconciles = defaultMaxPendingReconciles
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.

Comment thread pkg/api/v1/proto/keys/service.go Outdated
var parentID *string
if req.GetParentId() != "" {
p := req.GetParentId()
parentID = &p
Copy link
Copy Markdown
Contributor

@jithinkunjachan jithinkunjachan May 12, 2026

Choose a reason for hiding this comment

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

A new feature

Suggested change
parentID = &p
parentID = new(req.GetParentId())

Copy link
Copy Markdown
Contributor

@fabenan-f fabenan-f left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants