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

Golang SDK #218

Merged
merged 41 commits into from
Nov 30, 2023
Merged

Golang SDK #218

merged 41 commits into from
Nov 30, 2023

Conversation

cvele
Copy link
Contributor

@cvele cvele commented Sep 4, 2023

Type of change

- [ ] Bug fix
- [ x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

The objective is to develop a Golang wrapper for the Bitwarden client that interacts with a native rust library. This is to ensure that a consistent interface is provided, and the details of the Bitwarden rust library can be abstracted away.

Code changes

BitwardenClient.go

Defines the BitwardenClient struct with dependencies and submodules for handling projects and secrets.
Implements the NewBitwardenClient function that creates a new client.
Implements methods for access token login and cleanup.

BitwardenLibrary_native.go

Implements the BitwardenLibrary interface using cgo for interacting with the native C library.
Provides methods for initialization, memory release, and running commands on the client.

BitwardenLibrary_custom.go

An alternative implementation of the BitwardenLibrary interface, possibly for testing or alternative backends. The code is currently the same as BitwardenLibrary_native.go.

CommandRunner.go

Implements the CommandRunner struct that takes care of running commands via the library interface.
Centralizes command-running logic.

Projects.go

Implements a Projects struct with methods to create, list, get, update, and delete projects.
Utilizes the CommandRunner interface to run these commands.

Secrets.go

Implements a Secrets struct with methods to create, list, get, update, and delete secrets.
Utilizes the CommandRunner interface to run these commands.

Notes

  • The code is designed to be modular, separating concerns into different components (like Projects, Secrets, and CommandRunner).
  • Interfaces are used where possible to make it easier to mock these components for testing.
  • The native C API is encapsulated neatly, making it easier to switch out or update the underlying library without affecting the rest of the code.

@cvele cvele changed the title WIP - Golang SDK Golang SDK Sep 5, 2023
Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

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

I think we should look into moving the c interface behind an internal package since it's not "safe" to consume. And only expose managed wrappers.

Do we need to define a proper package instead of main? Can golang packages be distributed as a subdirectory of this repo or do we need to host the code in it's own repository.

languages/go/README.md Outdated Show resolved Hide resolved
languages/go/README.md Show resolved Hide resolved
support/scripts/schemas.ts Outdated Show resolved Hide resolved
languages/go/main.go Outdated Show resolved Hide resolved
languages/go/main.go Outdated Show resolved Hide resolved
languages/go/main.go Outdated Show resolved Hide resolved
@cvele cvele requested a review from a team as a code owner October 2, 2023 07:03
@cvele
Copy link
Contributor Author

cvele commented Oct 2, 2023

I think we should look into moving the c interface behind an internal package since it's not "safe" to consume. And only expose managed wrappers.

👍

Do we need to define a proper package instead of main? Can golang packages be distributed as a subdirectory of this repo or do we need to host the code in it's own repository.

You don't necessarily need a separate repository for distributing your Go package. You can have it as a subdirectory in your existing repository. In your case, with the configuration as module github.com/bitwarden/sdk/languages/go, the package can reside as a subdirectory under this path.

Consumers of your package would import it like this: import "github.com/bitwarden/sdk/languages/go"

languages/go/project.go Outdated Show resolved Hide resolved
@dani-garcia
Copy link
Member

Some small comments:

  • the names of the files in internal/cinterface are mispelled: libary -> library.
  • with the move to internal/cinterface, the build.sh script needs to update it's path now, at the moment it's expecting bitwarden_library.go in the same directory.
  • I think we should consider moving the example to a separate example directory to avoid mixing it with the library code. We are doing that as well in the .NET implementation. I've done that and made the example into a proper go module so I could test the changes locally (here's the commit 0eb862f), what do you think about doing something like that? . In ClientSettings I had to add & to the arguments otherwise it wouldn't compile, not sure what's up with that.

Otherwise the code worked correctly in my testing and I see you're already working on the CI release action, great!

@dirtycajunrice
Copy link

Can we get a condensed list of what is still blocking merge? The inclusion of this SDK is the only thing blocking bitwarden's inclusion in external secrets for k8s external-secrets/external-secrets#2661

@cvele
Copy link
Contributor Author

cvele commented Oct 26, 2023

Some small comments:

* the names of the files in `internal/cinterface` are mispelled: `libary` -> `library`.

* with the move to `internal/cinterface`, the `build.sh` script needs to update it's path now, at the moment it's expecting `bitwarden_library.go` in the same directory.

* I think we should consider moving the example to a separate `example` directory to avoid mixing it with the library code. We are doing that as well in the .NET implementation. I've done that and made the example into a proper go module so I could test the changes locally (here's the commit [0eb862f](https://github.com/bitwarden/sdk/commit/0eb862f50a24f85073f63a3daf71582a3004c656)), what do you think about doing something like that? . In `ClientSettings` I had to add `&` to the arguments otherwise it wouldn't compile, not sure what's up with that.

Otherwise the code worked correctly in my testing and I see you're already working on the CI release action, great!

Sorry for a delay, but all issues have been addressed. Additionally removed hard dependancy on clientsettings from schemas.

Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

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

Minor nit and please run prettier on the readme. npm run prettier in the project root.

Otherwise it looks good!

languages/go/internal/cinterface/bitwarden_library.go Outdated Show resolved Hide resolved
@bbujak
Copy link
Contributor

bbujak commented Nov 27, 2023

Minor nit and please run prettier on the readme. npm run prettier in the project root.

Otherwise it looks good!

I did run it , but no changes

@dani-garcia
Copy link
Member

The languages directories are excluded from prettier in .prettierignore, it seems. To format the README you can instead do:

cd languages/go
npx prettier --write README.md

We'll need to fix the .prettierignore in the future.

@bbujak
Copy link
Contributor

bbujak commented Nov 28, 2023

The languages directories are excluded from prettier in .prettierignore, it seems. To format the README you can instead do:

cd languages/go
npx prettier --write README.md

We'll need to fix the .prettierignore in the future.

That did the trick

dani-garcia
dani-garcia previously approved these changes Nov 28, 2023
Hinton
Hinton previously approved these changes Nov 28, 2023
vgrassia
vgrassia previously approved these changes Nov 28, 2023
@bbujak bbujak dismissed stale reviews from vgrassia, Hinton, and dani-garcia via 1dc8a3b November 30, 2023 08:14
@dani-garcia dani-garcia merged commit 9f77596 into bitwarden:master Nov 30, 2023
49 of 52 checks passed
@larivierec
Copy link

hello I know the PR is already closed but I have a small question.
Is the go package going to include the rust generated binary?

i see a mention of libbitwarden_c.
this is the main reason i'm asking

@Hinton
Copy link
Member

Hinton commented Dec 5, 2023

@larivierec Our plan is for go users to download the c-lib manually since including it in the repository would be unmaintainable and quickly balloon the repository size.

@carnei-ro
Copy link
Contributor

@cvele hey man, I'm having a bad time trying to compile the example project located at ./languages/go/example

# github.com/bitwarden/sdk/languages/go
../command_runner.go:10:21: undefined: Command
../project.go:4:47: undefined: ProjectResponse
../project.go:5:32: undefined: ProjectsResponse
../project.go:6:26: undefined: ProjectResponse
../project.go:7:65: undefined: ProjectResponse
../project.go:8:32: undefined: ProjectsDeleteResponse
../secrets.go:4:80: undefined: SecretResponse
../secrets.go:5:32: undefined: SecretIdentifiersResponse
../secrets.go:6:25: undefined: SecretResponse
../secrets.go:7:97: undefined: SecretResponse
../secrets.go:7:97: too many errors

any tips?

@Hinton
Copy link
Member

Hinton commented Dec 11, 2023

@carnei-ro you need to run npm run schemas in the repository root to generate the schema definitions.

@carnei-ro
Copy link
Contributor

carnei-ro commented Dec 11, 2023

@Hinton now I have this error:

leo@mac:/private/tmp/bitwarden/sdk/languages/go/example$ go build
# example
/Users/leo/.asdf/installs/golang/1.19/go/pkg/tool/darwin_arm64/link: running clang failed: exit status 1
ld: warning: directory not found for option '-L/private/tmp/bitwarden/sdk/languages/go/internal/cinterface/lib'
ld: library not found for -lbitwarden_c
clang: error: linker command failed with exit code 1 (use -v to see invocation)

edit: If I run cp -vpr ../../../target/debug ../internal/cinterface/lib; go build it works

@Skarlso
Copy link

Skarlso commented Jun 12, 2024

I was trying to do this too and built the whole thing and did the vpr thing, but I'm getting

ld: symbol(s) not found for architecture x86_64

I'm new to cgo, so I'm not sure what's up. Any ideas anyone? I know this is a closed PR, but I tried the rest of the places, and there are no answers, guides, docs, anything on how to deal with the SDK.

@Skarlso
Copy link

Skarlso commented Jun 12, 2024

ah.... finally, success

export CGO_LDFLAGS="-framework CoreFoundation"
➜  example git:(main) ✗ go build example.go
➜  example git:(main) ✗ ls -l
total 22792
-rwxr-xr-x@ 1 skarlso  staff  11653184 Jun 12 13:31 example
-rw-r--r--@ 1 skarlso  staff      2802 Jun 12 12:25 example.go
-rw-r--r--@ 1 skarlso  staff       188 Jun 12 12:25 go.mod
-rw-r--r--@ 1 skarlso  staff       187 Jun 12 12:25 go.sum
➜  example git:(main) ✗ ./example
panic: API error: Access token is not in a valid format: Doesn't contain a decryption key
... don't care about this, the binary works.

@Skarlso
Copy link

Skarlso commented Jun 12, 2024

In fact, there was no need for the cp -vpr. Just simply export CGO_LDFLAGS="-framework CoreFoundation" and the thing built out of the box.

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.