Skip to content

Conversation

@Mujib-Ahasan
Copy link

Summary

The default value of machine_accounts flag is false now. But related functionalities sets it to true. This PR is raised for the release of the flag.

Fixes #5435

@Mujib-Ahasan Mujib-Ahasan requested a review from a team as a code owner November 15, 2025 19:09
@evankanderson
Copy link
Member

The test failure appears to be valid:

❌ TestAssignRole/grant_permission_to_GitHub_Action (0s)
      handlers_authz_test.go:894: 
          	Error Trace:	/home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:894
          	Error:      	Received unexpected error:
          	            	Code: 12
          	            	Name: UNIMPLEMENTED
          	            	Description: Unimplemented
          	            	Details: machine accounts are not enabled
          	Test:       	TestAssignRole/grant_permission_to_GitHub_Action
      controller.go:251: missing call(s) to *mock_roles.MockRoleService.CreateRoleAssignment(is anything, is anything, is anything, is anything, is equal to {repo:mindersec/community:ref:refs/heads/main  githubactions  } (auth.Identity), is equal to admin (authz.Role)) /home/runner/work/minder/minder/internal/controlplane/handlers_authz_test.go:839
      controller.go:251: aborting test due to missing call(s)

return nil, util.UserVisibleError(codes.Unimplemented, "human users may only be added by invitation")
}
if isMachine && !flags.Bool(ctx, s.featureFlags, flags.MachineAccounts) {
if isMachine {
Copy link
Member

Choose a reason for hiding this comment

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

Ooh, this is tricky, since this is a negative use of the flag -- in this case, I think flags.Bool(ctx, s.featureFlags, flags.MachineAccounts) should effectively always return true, so if isMachine && !true will always evaluate to false, and the entire if false {...} block can be removed.

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.

Remove machine_accounts feature flag

2 participants