Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .github/workflows/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -547,23 +547,25 @@ jobs:
cukes_platform_report.log
retention-days: 1

# test latest otdfctl CLI 'main' against platform PR branch
# test otdfctl CLI e2e against platform PR branch
otdfctl-test:
permissions:
contents: read
name: otdfctl e2e tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
persist-credentials: false
- name: Install GNU parallel
run: |
sudo apt update
sudo apt install -y parallel
- uses: opentdf/platform/test/start-up-with-containers@main

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: why move to local?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since we now explicitly check out the repo for the local e2e otdfctl step, we can use the local version of the action too. Otherwise, we'd have two checkouts (including the implicit one when you do use: *@main) and we wouldn't be able to test start-up-with-containers action changes (not relevant here). Down to revert this specific change in my next PR to the stacked branch if someone has a strong opinion.

- uses: ./test/start-up-with-containers
with:
platform-ref: ${{ github.event.pull_request.head.sha || github.sha }}
- uses: opentdf/otdfctl/e2e@main
with:
otdfctl-ref: "main"
- uses: ./otdfctl/e2e

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.

Will this change to opentdf/platform/otdfctl@main after mergning?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking not — to ensure breaking changes are caught before merging to main.

env:
TESTRAIL_USER: ${{ secrets.TESTRAIL_USER }}
TESTRAIL_PASS: ${{ secrets.TESTRAIL_PASS }}
Expand Down
18 changes: 6 additions & 12 deletions .github/workflows/nightly-checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:
check-latest: false
cache-dependency-path: |
platform/examples/go.sum
platform/otdfctl/go.sum
platform/protocol/go/go.sum
platform/sdk/go.sum
platform/service/go.sum
Expand Down Expand Up @@ -60,18 +61,11 @@ jobs:
wait-for: 90s
working-directory: platform

######## CHECKOUT/BUILD 'otdfctl' #############
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
with:
repository: opentdf/otdfctl
ref: main
fetch-depth: 0
path: otdfctl
persist-credentials: false
- run: go build -o otdfctl
working-directory: otdfctl
- run: cp otdfctl ../platform
working-directory: otdfctl
######## BUILD 'otdfctl' (now part of platform monorepo) #############
- run: go build -o otdfctl .
working-directory: platform/otdfctl
- run: cp otdfctl ../
working-directory: platform/otdfctl

######## RUN TESTS #############
- run: ./.github/scripts/connectivity-test.sh
Expand Down
23 changes: 23 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,31 @@ linters:
- linters:
- goimport
text: http://www.apache.org/licenses/LICENSE-2.0
# otdfctl: defer refactoring-level lint fixes to follow-up
- path: otdfctl/
linters:
- contextcheck
text: should pass the context parameter
- path: otdfctl/
linters:
- revive
text: unused-parameter
- path: otdfctl/
linters:
- revive
text: unexported-return
- path: otdfctl/
linters:
- revive
text: var-naming
- path: otdfctl/
linters:
- nolintlint
text: "exhaustive"
paths:
- .*\.pb\.go
- .*\.pb\.gw.go
- otdfctl/tui/ # excluded during migration, matching original otdfctl lint config
- third_party$
- builtin$
- examples$
Expand All @@ -204,6 +226,7 @@ formatters:
paths:
- .*\.pb\.go
- .*\.pb\.gw.go
- otdfctl/tui/
- third_party$
- builtin$
- examples$
10 changes: 6 additions & 4 deletions otdfctl/cmd/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ func InitProfile(c *cli.Cli) *profiles.OtdfctlProfileStore {

hasKeyringStore, err := osprofiles.HasGlobalStore(config.AppName, osprofiles.WithKeyringStore())
if err != nil {
slog.Warn("Could not determine whether any profiles were stored on the keyring, defaulting to filesystem.", "error", err)
slog.Warn("could not determine whether any profiles were stored on the keyring, defaulting to filesystem",
slog.Any("error", err),
)
}
if hasKeyringStore {
slog.Debug("Keyring store still active, migrating profiles to filesystem.")
slog.Debug("keyring store still active, migrating profiles to filesystem")
err := profiles.Migrate(profiles.ProfileDriverFileSystem, profiles.ProfileDriverKeyring)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Error during profile migration from %s, to %s. %s cannot continue with profiles being stored within %s, please use the `profile migrate` command to manually migrate profiles", profiles.ProfileDriverKeyring, profiles.ProfileDriverFileSystem, config.AppName, profiles.ProfileDriverKeyring), err)
Expand All @@ -67,12 +69,12 @@ func InitProfile(c *cli.Cli) *profiles.OtdfctlProfileStore {
profileName = defaultProfileName
}

slog.Debug("Using profile", "profile", profileName)
slog.Debug("using profile", slog.String("profile", profileName))

// load profile
store, err := profiles.LoadOtdfctlProfileStore(profiles.ProfileDriverFileSystem, profileName)
if err != nil {
c.ExitWithError(fmt.Sprintf("Failed to load profile: %s", profileName), err)
c.ExitWithError("Failed to load profile: "+profileName, err)
}

applyOutputFormatPreference(c, store)
Expand Down
4 changes: 2 additions & 2 deletions otdfctl/cmd/execute.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cmd

import (
"fmt"
"errors"
"os"

"github.com/opentdf/platform/otdfctl/pkg/cli"
Expand Down Expand Up @@ -52,7 +52,7 @@ func Execute(opts ...ExecuteOptFunc) {

func MountRoot(newRoot *cobra.Command, cmd *cobra.Command) error {
if newRoot == nil {
return fmt.Errorf("newRoot is nil")
return errors.New("newRoot is nil")
}

if cmd != nil {
Expand Down
4 changes: 2 additions & 2 deletions otdfctl/cmd/policy/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ func policyGetAction(cmd *cobra.Command, args []string) {

action, err := h.GetAction(cmd.Context(), id, name, namespace)
if err != nil {
identifier := fmt.Sprintf("id: %s", id)
identifier := "id: " + id
if id == "" {
identifier = fmt.Sprintf("name: %s", name)
identifier = "name: " + name
}
errMsg := fmt.Sprintf("Failed to find action (%s)", identifier)
cli.ExitWithError(errMsg, err)
Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/policy/attributeValues.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func filterValuesByState(values []*policy.Value, state policycommon.ActiveStateE
func paginateValues(values []*policy.Value, limit, offset int32) ([]*policy.Value, *policy.PageResponse) {
total := len(values)
pagination := &policy.PageResponse{
Total: int32(min(total, math.MaxInt32)), //nolint:gosec // bounded by min
Total: int32(min(total, math.MaxInt32)),
CurrentOffset: offset,
}

Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/policy/kasKeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ func policyUnsafeDeleteKasKey(cmd *cobra.Command, args []string) {
kasURI := c.Flags.GetRequiredString("kas-uri")
force := c.Flags.GetOptionalBool("force")

cli.ConfirmAction(cli.ActionDelete, fmt.Sprintf("key with kas uri: %s, and key identifier: %s", kasURI, kid), fmt.Sprintf("Id: %s", id), force)
cli.ConfirmAction(cli.ActionDelete, "key with kas uri: "+kasURI+", and key identifier: "+kid, "Id: "+id, force)

key, err := h.UnsafeDeleteKasKey(ctx, id, kid, kasURI)
if err != nil {
Expand Down
6 changes: 2 additions & 4 deletions otdfctl/cmd/policy/kasRegistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import (
"github.com/spf13/cobra"
)

var (
KasRegistryCmd = man.Docs.GetCommand("policy/kas-registry")
)
var KasRegistryCmd = man.Docs.GetCommand("policy/kas-registry")

func getKeyAccessRegistry(cmd *cobra.Command, args []string) {
c := cli.New(cmd, args)
Expand Down Expand Up @@ -77,7 +75,7 @@ func listKeyAccessRegistries(cmd *cobra.Command, args []string) {
)
rows := []table.Row{}
for _, kas := range resp.GetKeyAccessServers() {
//TODO: Remove in next release
// TODO: Remove in next release
key := policy.PublicKey{}
key.PublicKey = &policy.PublicKey_Cached{Cached: kas.GetPublicKey().GetCached()}
if kas.GetPublicKey().GetRemote() != "" {
Expand Down
4 changes: 1 addition & 3 deletions otdfctl/cmd/policy/keyManagementProvider.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package policy

import (
"fmt"

"github.com/evertras/bubble-table/table"
"github.com/opentdf/platform/otdfctl/cmd/common"
"github.com/opentdf/platform/otdfctl/pkg/cli"
Expand Down Expand Up @@ -163,7 +161,7 @@ func deleteProviderConfig(cmd *cobra.Command, args []string) {
cli.ExitWithError("Failed to get provider config", err)
}

cli.ConfirmAction(cli.ActionDelete, fmt.Sprintf("key provider config with id: %s", id), fmt.Sprintf("Provider Name: %s", pc.GetName()), force)
cli.ConfirmAction(cli.ActionDelete, "key provider config with id: "+id, "Provider Name: "+pc.GetName(), force)

err = h.DeleteProviderConfig(c.Context(), id)
if err != nil {
Expand Down
15 changes: 7 additions & 8 deletions otdfctl/cmd/policy/obligations.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ package policy
import (
"encoding/json"
"fmt"
"strings"

"strconv"
"strings"

"github.com/evertras/bubble-table/table"
"github.com/opentdf/platform/otdfctl/cmd/common"
Expand Down Expand Up @@ -69,11 +68,11 @@ func policyGetObligation(cmd *cobra.Command, args []string) {

obl, err := h.GetObligation(cmd.Context(), id, fqn)
if err != nil {
identifier := fmt.Sprintf("id: %s", id)
identifier := "id: " + id
if id == "" {
identifier = fmt.Sprintf("fqn: %s", fqn)
identifier = "fqn: " + fqn
}
errMsg := fmt.Sprintf("Failed to find obligation (%s)", identifier)
errMsg := "Failed to find obligation (" + identifier + ")"
cli.ExitWithError(errMsg, err)
}

Expand Down Expand Up @@ -248,11 +247,11 @@ func policyGetObligationValue(cmd *cobra.Command, args []string) {

value, err := h.GetObligationValue(cmd.Context(), id, fqn)
if err != nil {
identifier := fmt.Sprintf("id: %s", id)
identifier := "id: " + id
if id == "" {
identifier = fmt.Sprintf("fqn: %s", fqn)
identifier = "fqn: " + fqn
}
errMsg := fmt.Sprintf("Failed to find obligation value (%s)", identifier)
errMsg := "Failed to find obligation value (" + identifier + ")"
cli.ExitWithError(errMsg, err)
}

Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
var (
metadataLabels []string
defaultListFlagLimit int32 = 300
defaultListFlagOffset int32 = 0
defaultListFlagOffset int32

Cmd = &cobra.Command{
Use: man.Docs.GetDoc("policy").Use,
Expand Down
12 changes: 6 additions & 6 deletions otdfctl/cmd/policy/registeredResources.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ func policyGetRegisteredResource(cmd *cobra.Command, args []string) {

resource, err := h.GetRegisteredResource(cmd.Context(), id, name, namespace)
if err != nil {
identifier := fmt.Sprintf("id: %s", id)
identifier := "id: " + id
if id == "" {
identifier = fmt.Sprintf("name: %s", name)
identifier = "name: " + name
}
errMsg := fmt.Sprintf("Failed to find registered resource (%s)", identifier)
errMsg := "Failed to find registered resource (" + identifier + ")"
cli.ExitWithError(errMsg, err)
}

Expand Down Expand Up @@ -263,11 +263,11 @@ func policyGetRegisteredResourceValue(cmd *cobra.Command, args []string) {

value, err := h.GetRegisteredResourceValue(cmd.Context(), id, fqn)
if err != nil {
identifier := fmt.Sprintf("id: %s", id)
identifier := "id: " + id
if id == "" {
identifier = fmt.Sprintf("fqn: %s", fqn)
identifier = "fqn: " + fqn
}
errMsg := fmt.Sprintf("Failed to find registered resource value (%s)", identifier)
errMsg := "Failed to find registered resource value (" + identifier + ")"
cli.ExitWithError(errMsg, err)
}

Expand Down
4 changes: 1 addition & 3 deletions otdfctl/cmd/policy/resourceMappingGroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import (
"github.com/spf13/cobra"
)

var (
policyResourceMappingGroupsCmd *cobra.Command
)
var policyResourceMappingGroupsCmd *cobra.Command

func policyCreateResourceMappingGroup(cmd *cobra.Command, args []string) {
c := cli.New(cmd, args)
Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/policy/resourceMappings.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package policy

import (
_ "embed"
_ "embed" // required for go:embed directives
"fmt"
"strings"

Expand Down
8 changes: 4 additions & 4 deletions otdfctl/cmd/policy/subjectConditionSets.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ func createSubjectConditionSet(cmd *cobra.Command, args []string) {
if ssFileJSON != "" {
jsonFile, err := os.Open(ssFileJSON)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Failed to open file at path: %s", ssFileJSON), err)
cli.ExitWithError("Failed to open file at path: "+ssFileJSON, err)
}
defer jsonFile.Close()

bytes, err := io.ReadAll(jsonFile)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Failed to read bytes from file at path: %s", ssFileJSON), err)
cli.ExitWithError("Failed to read bytes from file at path: "+ssFileJSON, err)
}
ssBytes = bytes
} else {
Expand Down Expand Up @@ -208,13 +208,13 @@ func updateSubjectConditionSet(cmd *cobra.Command, args []string) {
if ssFileJSON != "" {
jsonFile, err := os.Open(ssFileJSON)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Failed to open file at path: %s", ssFileJSON), err)
cli.ExitWithError("Failed to open file at path: "+ssFileJSON, err)
}
defer jsonFile.Close()

bytes, err := io.ReadAll(jsonFile)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Failed to read bytes from file at path: %s", ssFileJSON), err)
cli.ExitWithError("Failed to read bytes from file at path: "+ssFileJSON, err)
}
ssBytes = bytes
} else {
Expand Down
8 changes: 4 additions & 4 deletions otdfctl/cmd/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,14 @@ var profileListCmd = &cobra.Command{
defaultProfile := globalCfg.GetDefaultProfile()

var sb strings.Builder
sb.WriteString(fmt.Sprintf("Listing profiles from %s\n", driverType))
fmt.Fprintf(&sb, "Listing profiles from %s\n", driverType)

for _, p := range osprofiles.ListProfiles(profiler) {
if p == defaultProfile {
sb.WriteString(fmt.Sprintf("* %s\n", p))
fmt.Fprintf(&sb, "* %s\n", p)
continue
}
sb.WriteString(fmt.Sprintf(" %s\n", p))
fmt.Fprintf(&sb, " %s\n", p)
}

c.ExitWithMessage(sb.String(), cli.ExitCodeSuccess)
Expand All @@ -127,7 +127,7 @@ var profileGetCmd = &cobra.Command{
driverType := getDriverTypeFromUser(c)
profileStore, err := profiles.LoadOtdfctlProfileStore(driverType, profileName)
if err != nil {
cli.ExitWithError(fmt.Sprintf("Error loading profile store for profile %s", profileName), err)
cli.ExitWithError("Error loading profile store for profile "+profileName, err)
}

isDefault := "false"
Expand Down
7 changes: 6 additions & 1 deletion otdfctl/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ func init() {
}

version := fmt.Sprintf("%s version %s (%s) %s", config.AppName, config.Version, config.BuildTime, config.CommitSha)
slog.Debug(version)
slog.Debug("otdfctl version",
slog.String("app", config.AppName),
slog.String("version", config.Version),
slog.String("build_time", config.BuildTime),
slog.String("commit_sha", config.CommitSha),
)
c.ExitWith(version, v, cli.ExitCodeSuccess, os.Stdout)
return
}
Expand Down
Loading
Loading