ronantakizawa/codereview-bench huggingface dataset:
Repository: hashicorp/nomad
File: command/agent/retry_join.go (Go)

Code before:
}

// RetryJoin is used to handle retrying a join until it succeeds or all retries
// are exhausted.
func (r *retryJoiner) RetryJoin() {
	if len(r.joinCfg.RetryJoin) == 0 {
		return
	}

	attempt := 0

	addrsToJoin := strings.Join(r.joinCfg.RetryJoin, " ")
	r.logger.Info("starting retry join", "servers", addrsToJoin)

	for {
		var (
			addrs, servers []string
			err            error
		)

		for _, addr := range r.joinCfg.RetryJoin {

			// If auto-discovery returns an error, log the error and
			// fall-through, so we reach the retry logic and loop back around
			// for another go.
			servers, err = r.autoDiscover.Addrs(addr, r.logger)
			if err != nil {
				r.logger.Error("discovering join addresses failed", "join_config", addr, "error", err)
			} else {
				addrs = append(addrs, servers...)
			}
		}

		if len(addrs) > 0 && r.joinFunc != nil {
			var numJoined int
			numJoined, err = r.joinFunc(addrs)
			if err == nil {
				r.logger.Info("retry join completed", "initial_servers", numJoined)
				return
			}
		}

		attempt++
		if r.joinCfg.RetryMaxAttempts > 0 && attempt > r.joinCfg.RetryMaxAttempts {
			r.logger.Error("max join retry exhausted, exiting")
			close(r.errCh)
			return
		}

		if err != nil {
			r.logger.Warn("join failed", "error", err, "retry", r.joinCfg.RetryInterval)

Review comment (bug):
By not shadowing here I think we end up logging twice unless at least one `RetryJoin` returned an address. We'll log the error here and then again for the `join failed` warning. But maybe that's ok?

Code after:
}

// RetryJoin is used to handle retrying a join until it succeeds or all retries
// are exhausted.
func (r *retryJoiner) RetryJoin() {
	if len(r.joinCfg.RetryJoin) == 0 {
		return
	}

	attempt := 0

	addrsToJoin := strings.Join(r.joinCfg.RetryJoin, " ")
	r.logger.Info("starting retry join", "servers", addrsToJoin)

	for {
		var (
			addrs []string
			err   error
		)

		for _, addr := range r.joinCfg.RetryJoin {

			// If auto-discovery returns an error, log the error and
			// fall-through, so we reach the retry logic and loop back around
			// for another go.
			servers, err := r.autoDiscover.Addrs(addr, r.logger)
			if err != nil {
				r.logger.Error("discovering join addresses failed", "join_config", addr, "error", err)
			} else {
				addrs = append(addrs, servers...)
			}
		}

		if len(addrs) > 0 && r.joinFunc != nil {
			var numJoined int
			numJoined, err = r.joinFunc(addrs)
			if err == nil {
				r.logger.Info("retry join completed", "initial_servers", numJoined)
				return
			}
		}

		if err != nil {
			r.logger.Warn("join failed", "error", err, "retry", r.joinCfg.RetryInterval)
		}

		attempt++
		if r.joinCfg.RetryMaxAttempts > 0 && attempt > r.joinCfg.RetryMaxAttempts {
			r.logger.Error("max join retry exhausted, exiting")
			close(r.errCh)
			return
Repository: hashicorp/nomad
File: command/agent/retry_join.go (Go)

Code before:

	addrsToJoin := strings.Join(r.joinCfg.RetryJoin, " ")
	r.logger.Info("starting retry join", "servers", addrsToJoin)

	for {
		var (
			addrs []string
			err   error
		)

		for _, addr := range r.joinCfg.RetryJoin {

			// If auto-discovery returns an error, log the error and
			// fall-through, so we reach the retry logic and loop back around
			// for another go.
			servers, err := r.autoDiscover.Addrs(addr, r.logger)
			if err != nil {
				r.logger.Error("discovering join addresses failed", "join_config", addr, "error", err)
			} else {
				addrs = append(addrs, servers...)
			}
		}

		if len(addrs) > 0 && r.joinFunc != nil {
			var numJoined int
			numJoined, err = r.joinFunc(addrs)
			if err == nil {
				r.logger.Info("retry join completed", "initial_servers", numJoined)
				return
			}
		}

		attempt++
		if r.joinCfg.RetryMaxAttempts > 0 && attempt > r.joinCfg.RetryMaxAttempts {
			r.logger.Error("max join retry exhausted, exiting")
			close(r.errCh)
			return
		}

		if err != nil {
			r.logger.Warn("join failed", "error", err, "retry", r.joinCfg.RetryInterval)
		}
		time.Sleep(r.joinCfg.RetryInterval)
	}
}


Review comment (suggestion):
Shouldn't the `if err != nil` check down on L205 be above the retry check on L198-203? Seems like the last error encountered still wouldn't get logged.

Code after:

	addrsToJoin := strings.Join(r.joinCfg.RetryJoin, " ")
	r.logger.Info("starting retry join", "servers", addrsToJoin)

	for {
		var (
			addrs []string
			err   error
		)

		for _, addr := range r.joinCfg.RetryJoin {

			// If auto-discovery returns an error, log the error and
			// fall-through, so we reach the retry logic and loop back around
			// for another go.
			servers, err := r.autoDiscover.Addrs(addr, r.logger)
			if err != nil {
				r.logger.Error("discovering join addresses failed", "join_config", addr, "error", err)
			} else {
				addrs = append(addrs, servers...)
			}
		}

		if len(addrs) > 0 && r.joinFunc != nil {
			var numJoined int
			numJoined, err = r.joinFunc(addrs)
			if err == nil {
				r.logger.Info("retry join completed", "initial_servers", numJoined)
				return
			}
		}

		if err != nil {
			r.logger.Warn("join failed", "error", err, "retry", r.joinCfg.RetryInterval)
		}

		attempt++
		if r.joinCfg.RetryMaxAttempts > 0 && attempt > r.joinCfg.RetryMaxAttempts {
			r.logger.Error("max join retry exhausted, exiting")
			close(r.errCh)
			return
		}

		time.Sleep(r.joinCfg.RetryInterval)
	}
}
Repository: hashicorp/nomad
File: client/commonplugins/secrets_plugin.go (Go)

Code before:
)

type SecretsPlugin interface {
	CommonPlugin
	Fetch(ctx context.Context, path string) (*SecretResponse, error)
}

type SecretResponse struct {
	Result map[string]string `json:"result"`
	Error  *string           `json:"error"`
}

type externalSecretsPlugin struct {
	logger log.Logger

	// pluginPath is the path on the host to the plugin executable
	pluginPath string

	// config is optional configuration passed via the secret
	// block's config parameter
	config map[string]string
}

// NewExternalSecretsPlugin creates an instance of a secrets plugin by validating the plugin
// binary exists and is executable, and parsing any string key/value pairs out of the config
// which will be used as environemtn variables for Fetch.
func NewExternalSecretsPlugin(commonPluginDir string, name string, config map[string]any) (*externalSecretsPlugin, error) {
	// validate plugin
	executable := filepath.Join(commonPluginDir, SecretsPluginDir, name)
	f, err := os.Stat(executable)
	if err != nil {
		if os.IsNotExist(err) {
			return nil, fmt.Errorf("%w: %q", ErrPluginNotExists, name)
		}
		return nil, err
	}
	if !helper.IsExecutable(f) {
		return nil, fmt.Errorf("%w: %q", ErrPluginNotExecutable, name)
	}

	// parse any string key/value pairs.
	conf := make(map[string]string)
	for k, v := range config {
		if s, ok := v.(string); ok {
			conf[k] = s
		}
	}

	return &externalSecretsPlugin{
		pluginPath: executable,
		config:     conf,

Review comment (suggestion):
```suggestion
// which will be used as environment variables for Fetch.
```

Code after:
)

type SecretsPlugin interface {
	CommonPlugin
	Fetch(ctx context.Context, path string) (*SecretResponse, error)
}

type SecretResponse struct {
	Result map[string]string `json:"result"`
	Error  *string           `json:"error"`
}

type externalSecretsPlugin struct {
	logger log.Logger

	// pluginPath is the path on the host to the plugin executable
	pluginPath string

	// env is optional envVars passed to the plugin process
	env map[string]string
}

// NewExternalSecretsPlugin creates an instance of a secrets plugin by validating the plugin
// binary exists and is executable, and parsing any string key/value pairs out of the config
// which will be used as environment variables for Fetch.
func NewExternalSecretsPlugin(commonPluginDir string, name string, env map[string]string) (*externalSecretsPlugin, error) {
	// validate plugin
	executable := filepath.Join(commonPluginDir, SecretsPluginDir, name)
	f, err := os.Stat(executable)
	if err != nil {
		if os.IsNotExist(err) {
			return nil, fmt.Errorf("%w: %q", ErrPluginNotExists, name)
		}
		return nil, err
	}
	if !helper.IsExecutable(f) {
		return nil, fmt.Errorf("%w: %q", ErrPluginNotExecutable, name)
	}

	return &externalSecretsPlugin{
		pluginPath: executable,
		env:        env,
	}, nil
}

func (e *externalSecretsPlugin) Fingerprint(ctx context.Context) (*PluginFingerprint, error) {
	plugCtx, cancel := context.WithTimeout(ctx, SecretsCmdTimeout)
	defer cancel()

	cmd := exec.CommandContext(plugCtx, e.pluginPath, "fingerprint")
	cmd.Env = []string{
Repository: hashicorp/nomad
File: client/allocrunner/taskrunner/secrets/plugin_provider.go (Go)

Code before:
	path string
}

type Response struct {
	Result map[string]string `json:"result"`
	Error  *string           `json:"error,omitempty"`
}

func NewExternalPluginProvider(plugin commonplugins.SecretsPlugin, name string, path string) *ExternalPluginProvider {
	return &ExternalPluginProvider{
		plugin: plugin,
		name:   name,
		path:   path,
	}
}

func (p *ExternalPluginProvider) Fetch(ctx context.Context) (map[string]string, error) {
	resp, err := p.plugin.Fetch(ctx, p.path)
	if err != nil {
		return nil, fmt.Errorf("failed to fetch secret from plugin %s: %w", p.name, err)
	}
	if resp.Error != nil {
		return nil, fmt.Errorf("error returned from secret plugin %s: %s", p.name, *resp.Error)
	}

	formatted := map[string]string{}
	for k, v := range resp.Result {
		formatted[fmt.Sprintf("secret.%s.%s", p.name, k)] = v
	}

	return formatted, nil
}


Review comment (suggestion):
We could possibly pre-allocate this map as we know the length of `resp.Result` but I don't believe it will make any difference in reality.

Code after:
	path string
}

type Response struct {
	Result map[string]string `json:"result"`
	Error  *string           `json:"error,omitempty"`
}

func NewExternalPluginProvider(plugin commonplugins.SecretsPlugin, name string, path string) *ExternalPluginProvider {
	return &ExternalPluginProvider{
		plugin: plugin,
		name:   name,
		path:   path,
	}
}

func (p *ExternalPluginProvider) Fetch(ctx context.Context) (map[string]string, error) {
	resp, err := p.plugin.Fetch(ctx, p.path)
	if err != nil {
		return nil, fmt.Errorf("failed to fetch secret from plugin %s: %w", p.name, err)
	}
	if resp.Error != nil {
		return nil, fmt.Errorf("error returned from secret plugin %s: %s", p.name, *resp.Error)
	}

	formatted := make(map[string]string, len(resp.Result))
	for k, v := range resp.Result {
		formatted[fmt.Sprintf("secret.%s.%s", p.name, k)] = v
	}

	return formatted, nil
}
Repository: hashicorp/nomad
File: scheduler/structs/structs.go (Go)

Code before:
	return c
}

func (ev *PortCollisionEvent) Sanitize() *PortCollisionEvent {
	if ev == nil {
		return nil
	}
	clean := ev.Copy()

	clean.Node = ev.Node.Sanitize()
	clean.Node.Meta = make(map[string]string)

	for i, alloc := range ev.Allocations {
		clean.Allocations[i] = alloc.CopySkipJob()
		clean.Allocations[i].Job = nil
	}

	return clean
}

// NewPlanWithSateAndIndex is used in the testing harness
func NewPlanWithSateAndIndex(state *state.StateStore, nextIndex uint64, serversMeetMinimumVersion bool) *Plan {
	return &Plan{State: state, nextIndex: nextIndex, serversMeetMinimumVersion: serversMeetMinimumVersion}
}

// Plan is used to submit plans.
type Plan struct {
	State *state.StateStore

	Planner  Planner
	planLock sync.Mutex

	Plans        []*structs.Plan
	Evals        []*structs.Evaluation
	CreateEvals  []*structs.Evaluation
	ReblockEvals []*structs.Evaluation

	nextIndex     uint64
	nextIndexLock sync.Mutex

	optimizePlan              bool
	serversMeetMinimumVersion bool

	// don't actually write plans back to state
	noSubmit bool
}

// SubmitPlan is used to handle plan submission
func (p *Plan) SubmitPlan(plan *structs.Plan) (*structs.PlanResult, State, error) {
	// Ensure sequential plan application
	p.planLock.Lock()

Review comment (refactor):
If we're refactoring this for the sake of cleanliness and not function anyways, can we put all this relocated code somewhere that isn't the 13k+ line `structs.go` file?

Code after:
	return c
}

func (ev *PortCollisionEvent) Sanitize() *PortCollisionEvent {
	if ev == nil {
		return nil
	}
	clean := ev.Copy()

	clean.Node = ev.Node.Sanitize()
	clean.Node.Meta = make(map[string]string)

	for i, alloc := range ev.Allocations {
		clean.Allocations[i] = alloc.CopySkipJob()
		clean.Allocations[i].Job = nil
	}

	return clean
}

// NewPlanWithStateAndIndex is used in the testing harness
func NewPlanWithStateAndIndex(state *state.StateStore, nextIndex uint64, serversMeetMinimumVersion bool) *PlanBuilder {
	return &PlanBuilder{State: state, nextIndex: nextIndex, serversMeetMinimumVersion: serversMeetMinimumVersion}
}

// PlanBuilder is used to submit plans.
type PlanBuilder struct {
	State *state.StateStore

	Planner  Planner
	planLock sync.Mutex

	Plans        []*structs.Plan
	Evals        []*structs.Evaluation
	CreateEvals  []*structs.Evaluation
	ReblockEvals []*structs.Evaluation

	nextIndex     uint64
	nextIndexLock sync.Mutex

	serversMeetMinimumVersion bool

	// don't actually write plans back to state
	noSubmit bool
}

// SubmitPlan is used to handle plan submission
func (p *PlanBuilder) SubmitPlan(plan *structs.Plan) (*structs.PlanResult, State, error) {
	// Ensure sequential plan application
	p.planLock.Lock()
	defer p.planLock.Unlock()
Repository: hashicorp/nomad
File: scheduler/structs/structs.go (Go)

Code before:
		}

	}
	c.NetIndex = ev.NetIndex.Copy()
	return c
}

func (ev *PortCollisionEvent) Sanitize() *PortCollisionEvent {
	if ev == nil {
		return nil
	}
	clean := ev.Copy()

	clean.Node = ev.Node.Sanitize()
	clean.Node.Meta = make(map[string]string)

	for i, alloc := range ev.Allocations {
		clean.Allocations[i] = alloc.CopySkipJob()
		clean.Allocations[i].Job = nil
	}

	return clean
}

// NewPlanWithSateAndIndex is used in the testing harness
func NewPlanWithSateAndIndex(state *state.StateStore, nextIndex uint64, serversMeetMinimumVersion bool) *Plan {
	return &Plan{State: state, nextIndex: nextIndex, serversMeetMinimumVersion: serversMeetMinimumVersion}
}

// Plan is used to submit plans.
type Plan struct {
	State *state.StateStore

	Planner  Planner
	planLock sync.Mutex

	Plans        []*structs.Plan
	Evals        []*structs.Evaluation
	CreateEvals  []*structs.Evaluation
	ReblockEvals []*structs.Evaluation

	nextIndex     uint64
	nextIndexLock sync.Mutex

	optimizePlan              bool
	serversMeetMinimumVersion bool

	// don't actually write plans back to state
	noSubmit bool
}


Review comment (suggestion):
```suggestion
func NewPlanWithStateAndIndex(state *state.StateStore, nextIndex uint64, serversMeetMinimumVersion bool) *Plan {
```

~Also, this constructor appears to be test-only. Maybe this belongs in the scheduler?~ Edit: :facepalm: 

Code after:
		}

	}
	c.NetIndex = ev.NetIndex.Copy()
	return c
}

func (ev *PortCollisionEvent) Sanitize() *PortCollisionEvent {
	if ev == nil {
		return nil
	}
	clean := ev.Copy()

	clean.Node = ev.Node.Sanitize()
	clean.Node.Meta = make(map[string]string)

	for i, alloc := range ev.Allocations {
		clean.Allocations[i] = alloc.CopySkipJob()
		clean.Allocations[i].Job = nil
	}

	return clean
}

// NewPlanWithStateAndIndex is used in the testing harness
func NewPlanWithStateAndIndex(state *state.StateStore, nextIndex uint64, serversMeetMinimumVersion bool) *PlanBuilder {
	return &PlanBuilder{State: state, nextIndex: nextIndex, serversMeetMinimumVersion: serversMeetMinimumVersion}
}

// PlanBuilder is used to submit plans.
type PlanBuilder struct {
	State *state.StateStore

	Planner  Planner
	planLock sync.Mutex

	Plans        []*structs.Plan
	Evals        []*structs.Evaluation
	CreateEvals  []*structs.Evaluation
	ReblockEvals []*structs.Evaluation

	nextIndex     uint64
	nextIndexLock sync.Mutex

	serversMeetMinimumVersion bool

	// don't actually write plans back to state
	noSubmit bool
}

// SubmitPlan is used to handle plan submission
Repository: hashicorp/nomad
File: client/state/db_test.go (Go)

Code before:
			must.MapContainsKeys(t, results, []string{"alloc2"})
			must.MapContainsKeys(t, results["alloc2"], []structs.CheckID{"id5"})
		})
	})

}

func TestStateDB_NodeIdentity(t *testing.T) {
	ci.Parallel(t)

	testDB(t, func(t *testing.T, db StateDB) {
		identity, err := db.GetNodeIdentity()
		must.NoError(t, err)
		must.Eq(t, "", identity)

		fakeIdentity := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"

		must.NoError(t, db.PutNodeIdentity(fakeIdentity))

		identity, err = db.GetNodeIdentity()
		must.NoError(t, err)
		must.Eq(t, fakeIdentity, identity)
	})
}

func TestStateDB_CnsulACLToken(t *testing.T) {
	ci.Parallel(t)

	testDB(t, func(t *testing.T, db StateDB) {
		alloc1 := mock.Alloc()

		must.NoError(t, db.PutAllocation(alloc1))
		tokens, err := db.GetAllocConsulACLTokens(alloc1.ID)
		must.NoError(t, err)
		must.Eq(t, nil, tokens)

		fakeToken := &structs.ConsulACLToken{
			WorkloadID: "workloadID",
			ACLToken:   "token",
		}

		must.NoError(t, db.PutAllocConsulACLTokens(alloc1.ID, []*structs.ConsulACLToken{fakeToken}))

		tokens, err = db.GetAllocConsulACLTokens(alloc1.ID)
		must.NoError(t, err)
		must.One(t, len(tokens))
		must.Eq(t, fakeToken, tokens[0])
	})
}

// TestStateDB_Upgrade asserts calling Upgrade on new databases always

Review comment (suggestion):
```suggestion
func TestStateDB_ConsulACLToken(t *testing.T) {
```

Code after:
			must.MapContainsKeys(t, results, []string{"alloc2"})
			must.MapContainsKeys(t, results["alloc2"], []structs.CheckID{"id5"})
		})
	})

}

func TestStateDB_NodeIdentity(t *testing.T) {
	ci.Parallel(t)

	testDB(t, func(t *testing.T, db StateDB) {
		identity, err := db.GetNodeIdentity()
		must.NoError(t, err)
		must.Eq(t, "", identity)

		fakeIdentity := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.KMUFsIDTnFmyG3nMiGM6H9FNFUROf3wh7SmqJp-QV30"

		must.NoError(t, db.PutNodeIdentity(fakeIdentity))

		identity, err = db.GetNodeIdentity()
		must.NoError(t, err)
		must.Eq(t, fakeIdentity, identity)
	})
}

func TestStateDB_ConsulACLToken(t *testing.T) {
	ci.Parallel(t)

	testDB(t, func(t *testing.T, db StateDB) {
		alloc1 := mock.Alloc()

		must.NoError(t, db.PutAllocation(alloc1))
		tokens, err := db.GetAllocConsulACLTokens(alloc1.ID)
		must.NoError(t, err)
		must.Eq(t, nil, tokens)

		fakeToken := &cstructs.ConsulACLToken{
			Cluster:  "fake cluster",
			TokenID:  "workloadID",
			ACLToken: "token",
		}

		must.NoError(t, db.PutAllocConsulACLTokens(alloc1.ID, []*cstructs.ConsulACLToken{fakeToken}))

		tokens, err = db.GetAllocConsulACLTokens(alloc1.ID)
		must.NoError(t, err)
		must.One(t, len(tokens))
		must.Eq(t, fakeToken, tokens[0])
	})
}
Repository: hashicorp/nomad
File: client/allocrunner/consul_hook.go (Go)

Code before:
	return &resourcesBackend{
		hookResources: hr,
		db:            db,
	}
}

func decodeACLToken(b64ACLToken string, token *consulapi.ACLToken) error {
	decodedBytes, err := base64.StdEncoding.DecodeString(b64ACLToken)
	if err != nil {
		return fmt.Errorf("unable to process ACLToken: %w", err)
	}

	if len(decodedBytes) != 0 {
		if err := json.Unmarshal(decodedBytes, token); err != nil {
			return fmt.Errorf("unable to unmarshal ACLToken: %w", err)
		}
	}

	return nil
}

func encodeACLToken(token *consulapi.ACLToken) (string, error) {
	jsonBytes, err := json.Marshal(token)
	if err != nil {
		return "", fmt.Errorf("unable to marshal ACL token: %w", err)
	}

	return base64.StdEncoding.EncodeToString(jsonBytes), nil
}

// This function will never return nil, even in case of error
func (rs *resourcesBackend) loadAllocTokens(allocID string) (map[string]map[string]*consulapi.ACLToken, error) {
	allocTokens := map[string]map[string]*consulapi.ACLToken{}

	ts, err := rs.db.GetAllocConsulACLTokens(allocID)
	if err != nil {
		return allocTokens, err
	}

	var mErr *multierror.Error
	for _, st := range ts {
		token := &consulapi.ACLToken{}
		err := decodeACLToken(st.ACLToken, token)
		if err != nil {
			mErr = multierror.Append(mErr, err)
			continue
		}

		allocTokens[st.Cluster][st.TokenID] = token
	}


Review comment (suggestion):
See my note on the new struct about how we can skip this encoding (and that'll eliminate some error handling)

Code after:
	}
}

func decodeACLToken(b64ACLToken string, token *consulapi.ACLToken) error {
	decodedBytes, err := base64.StdEncoding.DecodeString(b64ACLToken)
	if err != nil {
		return fmt.Errorf("unable to process ACLToken: %w", err)
	}

	if len(decodedBytes) != 0 {
		if err := json.Unmarshal(decodedBytes, token); err != nil {
			return fmt.Errorf("unable to unmarshal ACLToken: %w", err)
		}
	}

	return nil
}

func encodeACLToken(token *consulapi.ACLToken) (string, error) {
	jsonBytes, err := json.Marshal(token)
	if err != nil {
		return "", fmt.Errorf("unable to marshal ACL token: %w", err)
	}

	return base64.StdEncoding.EncodeToString(jsonBytes), nil
}

// This function will never return nil, even in case of error
func (rs *resourcesBackend) loadAllocTokens() (map[string]map[string]*consulapi.ACLToken, error) {
	allocTokens := map[string]map[string]*consulapi.ACLToken{}

	ts, err := rs.db.GetAllocConsulACLTokens(rs.allocID)
	if err != nil {
		return allocTokens, err
	}

	var mErr *multierror.Error
	for _, st := range ts {

		token := &consulapi.ACLToken{}
		err := decodeACLToken(st.ACLToken, token)
		if err != nil {
			mErr = multierror.Append(mErr, err)
			continue
		}

		if allocTokens[st.Cluster] == nil {
			allocTokens[st.Cluster] = map[string]*consulapi.ACLToken{}
		}

		allocTokens[st.Cluster][st.TokenID] = token