Extract permission node creation 8201

[tkalir]

Related to #8201

Change Description

Automating the documentation for endpoint permissions requires to decouple the permission objects from the endpoint code. This PR extracts the permission objects to a separate file.
Since the permission code is sensitive, I opted for a straightforward refactor as a first step. This also makes it possible to test the next changes in the permission code separately from the endpoint code.
The functions are named after the logAction operation names when available (e.g copy_objects -> CopyObjectsPermissions).

Testing Details

When refactoring I used the IDE's "Extract Method" feature for consistency and minimize room for error. I also ran the tests locally (excluding Hadoop related tests, which I had an issue with running locally).
Let me know if you think additional coverage is required.

[arielshaqed]

Hi @tkalir ,

THANKS! Overall this change looks fine. I am curious how this will continue. At some point you will need to strings from all of these permissions, and you will need something in memory that holds all usable operations, and the knowledge that it matches the list of all your funcs. This will still be an implementation issue.

As an alternative: it seems that you have a small number of truly different "*Permissions" funcs. Could we instead implement something like

func MakeGroupPermissions(action string) func(groupID string) permissions.Node {
	return func(groupID string) permissions.Node {
		return permissions.Node{
			Permission: Permission{
				Action:   action,
				Resource: GroupArn(groupID),
			},
		}
	}
}

// A "few" other similar Make*Permissions, say MakeObjectPermissions, MakePolicyPermissions, and similar.

Now you might store a Make*Permissions func in a map where the index is the operation name. And then you load in controller.go a permissions func, and then call that func. With a few tweaks we will be able to use that map to print documentation.

WDYT?

[arielshaqed]

This is a comment about the filename. Unfortunately filenames can be really tough. We dislike capitals in filenames for two reasons:

• The existing Go code base uses underscores not uppercase.

  Right now we have only one such file, which is auto-generated by a tool:

  ❯ find ./pkg/ ./cmd/ -name '*[A-Z]*.go'
  ./pkg/metastore/hive/gen-go/hive_metastore/GoUnusedProtection__.go

• Uppercase can actively be harmful because of bad defaults on MacOS. By default, MacOS creates a case-insensitive but case-preserving filesystem when you set it up. So this file will also be accessible by names operationpermissions.go, opeRATionPermissions.go, and others. But as soon as someone does that by mistake, every other OS fails to find that file. We have had bad experience here. So we try to avoid uppercase where it matters.

BUT please make this change only after we approve all other issues on the review. Otherwise GitHub tends to lose the comments when you rename a file. 😢

[tkalir]

Got it, thanks for the explanation. I'll rename the file after the rest is fine.

[tkalir]

@arielshaqed thanks for reviewing!

My initial thought was doing something similar to the draft here.
The tldr version is along the lines of mapping each operation name to a permissions object that has the resource template (e.g "group/{groupID}") in the resource field. The doc generation part will use this object as is, and when used in the endpoint flow it will call a "fill" function that substitutes the placeholders with the actual values.

If I understand your suggestion correctly, the doc-gen code is supposed to call the function that MakeGroupPermissions (etc) returns, which means it needs to know how many arguments this function expects. It's not impossible to solve, but I think the template solution is easier in this case.

Unrelated question - there's an automatic check that fails this PR for not closing an issue. I'd rather split the code to multiple PRs (I also want to extract the log_action strings to enums before making the map object, which I don't think belongs here). In that case, this PR won't close the original issue. Should I create another issue for this PR to close?
Thanks