Skip to content

Commit ec0c019

Browse files
authored
fix: modify credential refresh to support stacked contexts (#856)
Signed-off-by: Grant Linville <[email protected]>
1 parent 2eafb08 commit ec0c019

File tree

3 files changed

+57
-12
lines changed

3 files changed

+57
-12
lines changed

Diff for: pkg/credentials/noop.go

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ func (s NoopStore) Add(context.Context, Credential) error {
1212
return nil
1313
}
1414

15+
func (s NoopStore) Refresh(context.Context, Credential) error {
16+
return nil
17+
}
18+
1519
func (s NoopStore) Remove(context.Context, string) error {
1620
return nil
1721
}

Diff for: pkg/credentials/store.go

+21
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"path/filepath"
77
"regexp"
8+
"slices"
89
"strings"
910

1011
"github.com/docker/cli/cli/config/credentials"
@@ -26,6 +27,7 @@ type CredentialBuilder interface {
2627
type CredentialStore interface {
2728
Get(ctx context.Context, toolName string) (*Credential, bool, error)
2829
Add(ctx context.Context, cred Credential) error
30+
Refresh(ctx context.Context, cred Credential) error
2931
Remove(ctx context.Context, toolName string) error
3032
List(ctx context.Context) ([]Credential, error)
3133
}
@@ -95,6 +97,8 @@ func (s Store) Get(ctx context.Context, toolName string) (*Credential, bool, err
9597
return &cred, true, nil
9698
}
9799

100+
// Add adds a new credential to the credential store.
101+
// Any context set on the credential object will be overwritten with the first context of the credential store.
98102
func (s Store) Add(ctx context.Context, cred Credential) error {
99103
first := first(s.credCtxs)
100104
if first == AllCredentialContexts {
@@ -113,6 +117,23 @@ func (s Store) Add(ctx context.Context, cred Credential) error {
113117
return store.Store(auth)
114118
}
115119

120+
// Refresh updates an existing credential in the credential store.
121+
func (s Store) Refresh(ctx context.Context, cred Credential) error {
122+
if !slices.Contains(s.credCtxs, cred.Context) {
123+
return fmt.Errorf("context %q not in list of valid contexts for this credential store", cred.Context)
124+
}
125+
126+
store, err := s.getStore(ctx)
127+
if err != nil {
128+
return err
129+
}
130+
auth, err := cred.toDockerAuthConfig()
131+
if err != nil {
132+
return err
133+
}
134+
return store.Store(auth)
135+
}
136+
116137
func (s Store) Remove(ctx context.Context, toolName string) error {
117138
first := first(s.credCtxs)
118139
if len(s.credCtxs) > 1 || first == AllCredentialContexts {

Diff for: pkg/runner/runner.go

+32-12
Original file line numberDiff line numberDiff line change
@@ -854,8 +854,10 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env
854854
}
855855

856856
var (
857-
c *credentials.Credential
858-
exists bool
857+
c *credentials.Credential
858+
resultCredential credentials.Credential
859+
exists bool
860+
refresh bool
859861
)
860862

861863
rm := runtimeWithLogger(callCtx, monitor, r.runtimeManager)
@@ -886,6 +888,7 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env
886888
if !exists || c.IsExpired() {
887889
// If the existing credential is expired, we need to provide it to the cred tool through the environment.
888890
if exists && c.IsExpired() {
891+
refresh = true
889892
credJSON, err := json.Marshal(c)
890893
if err != nil {
891894
return nil, fmt.Errorf("failed to marshal credential: %w", err)
@@ -916,39 +919,56 @@ func (r *Runner) handleCredentials(callCtx engine.Context, monitor Monitor, env
916919
continue
917920
}
918921

919-
if err := json.Unmarshal([]byte(*res.Result), &c); err != nil {
922+
if err := json.Unmarshal([]byte(*res.Result), &resultCredential); err != nil {
920923
return nil, fmt.Errorf("failed to unmarshal credential tool %s response: %w", ref.Reference, err)
921924
}
922-
c.ToolName = credName
923-
c.Type = credentials.CredentialTypeTool
925+
resultCredential.ToolName = credName
926+
resultCredential.Type = credentials.CredentialTypeTool
927+
928+
if refresh {
929+
// If this is a credential refresh, we need to make sure we use the same context.
930+
resultCredential.Context = c.Context
931+
} else {
932+
// If it is a new credential, let the credential store determine the context.
933+
resultCredential.Context = ""
934+
}
924935

925936
isEmpty := true
926-
for _, v := range c.Env {
937+
for _, v := range resultCredential.Env {
927938
if v != "" {
928939
isEmpty = false
929940
break
930941
}
931942
}
932943

933-
if !c.Ephemeral {
944+
if !resultCredential.Ephemeral {
934945
// Only store the credential if the tool is on GitHub or has an alias, and the credential is non-empty.
935946
if (isGitHubTool(toolName) && callCtx.Program.ToolSet[ref.ToolID].Source.Repo != nil) || credentialAlias != "" {
936947
if isEmpty {
937948
log.Warnf("Not saving empty credential for tool %s", toolName)
938-
} else if err := r.credStore.Add(callCtx.Ctx, *c); err != nil {
939-
return nil, fmt.Errorf("failed to add credential for tool %s: %w", toolName, err)
949+
} else {
950+
if refresh {
951+
err = r.credStore.Refresh(callCtx.Ctx, resultCredential)
952+
} else {
953+
err = r.credStore.Add(callCtx.Ctx, resultCredential)
954+
}
955+
if err != nil {
956+
return nil, fmt.Errorf("failed to save credential for tool %s: %w", toolName, err)
957+
}
940958
}
941959
} else {
942960
log.Warnf("Not saving credential for tool %s - credentials will only be saved for tools from GitHub, or tools that use aliases.", toolName)
943961
}
944962
}
963+
} else {
964+
resultCredential = *c
945965
}
946966

947-
if c.ExpiresAt != nil && (nearestExpiration == nil || nearestExpiration.After(*c.ExpiresAt)) {
948-
nearestExpiration = c.ExpiresAt
967+
if resultCredential.ExpiresAt != nil && (nearestExpiration == nil || nearestExpiration.After(*resultCredential.ExpiresAt)) {
968+
nearestExpiration = resultCredential.ExpiresAt
949969
}
950970

951-
for k, v := range c.Env {
971+
for k, v := range resultCredential.Env {
952972
env = append(env, fmt.Sprintf("%s=%s", k, v))
953973
}
954974
}

0 commit comments

Comments
 (0)