feat(artifacts): Add ArtifactStore to Kork

[xibz]

This commit adds a new feature called ArtifactStore. This allows for storage of embedded/base64 types to some storage, eg S3. So instead of having duplication of some artifact in a context, it'll now be referenced by a artifact URI.

Also, there's a nice README that explains various settings as well as how to setup a local S3 for testing artifact storage

README link

The related PRs
spinnaker/clouddriver#5976
spinnaker/orca#4481
spinnaker/rosco#998
spinnaker/gate#1674

Further to enable this feature can be enabled via feature flag

artifactStore:
  enabled: true
  applicationsRegex: "only-these-applications-*" # optional regex to enable certain regex application names to have this enabled
  s3:
    enabled: true
    bucket: s3-artifact-bucket

We've also allowed for certain applications to have it enabled with shortens the blast radius of testing.

Also for some metrics, we saw large pipelines be reduced by 70%. It really comes down to how often an artifact is referenced in each stage. Simple smaller pipelines were reduced by 30%.

Without any formatting, the JSON file is 26,960,606 bytes in length, out of which over 20M are for the reference fields of embedded artifacts (almost exactly 75%).

---

TODO

• Create PR for Gate
• Create PR for Deck

[nemesisOsorio]

Interesting, what about deletion?

[xibz]

Yes, also works for deletion. You can think of the object lock as an SQL row lock

[nemesisOsorio]

I mean, what is the plan for deleting old artifacts in the bucket?

[xibz]

Ah,so S3 has built in cleanup of old objects. So whatever you use for infrastructure, eg pulumi, terraform, etc, you'd just need to set that when creating the bucket

[dbyron-sf]

Perhaps OldPipelineCleanupPollingNotificationAgent could have some additional logic to clean up artifacts when cleaning up old pipeline executions since the logic is more complex than "older than"? I think it's OK for that to happen in a followup PR.

[xibz]

Yea, it's just out of scope for S3 artifact stores since they have their own way of handling clean up. However, I do think if someone adds some storage that doesn't implement that, then some logic will be needed to handle that. I haven't thought too much on the design of that since that was outside of the scope, but am totally okay with some future PR to address this.

[nemesisOsorio]

I think we can move these lines outside of the for

[xibz]

Hmm, I don't think so. The for loop is iterating for each test case. From that test case we create a storage which then creates a serializer.

That serializer then is used to register itself to the newly created module which is then used to register itself to the newly created object mapper.

[nemesisOsorio]

should we move this method to the abstract class and enforce to check authorization? for future ArtifactStore implementations?

[xibz]

Hmm, potentially? I'll need to think about this some, cause it would require some movement of some stuff or may not change at all. Just requires some thinking here.

[xibz]

Also added gate PR, spinnaker/gate#1674

[dbyron-sf]

Pondering how helpful it would be to match the knobs clouddriver provides in S3ArtifactProviderProperties and S3ArtifactAccount.

[xibz]

Not a bad idea. I think originally I was trying to keep v2 of the AWS SDK as separate as possible, but went a little overboard with the config :p

[xibz]

We can refactor and instead look up the account based on some account name that is part of the artifact store properties. However, new fields would need to be added to the current S3ArtifactAccount class and a new account-name field to the artifact store configuration. We'd also need to see what is all compatible in configuration between V1 and V2 of the client configuration. I imagine this shouldn't be an issue.

This does require a little bit more work. Mostly restructuring things to be moved out of clouddriver into kork, and then adding the appropriate fields.

I feel like this can be a separate PR however. I would like to ensure that we implement it internally first so we don't deviate too far from upstream, if that works for you guys. Let me know your thoughts

[dbyron-sf]

That sounds good, and yeah, totally agree with not holding up this PR for it.

[sergio-quintero]

I wasn't able to build clouddriver locally using a local kork snapshot, but it built fine after removing this line. I think this is unnecessary?

[xibz]

Hmm, I built it just fine. I had to clear gradle cache however. I did use korkComposite=true in gradle.properties

[nemesisOsorio]

seems like software.amazon.awssdk:s3 is duplicated

[nemesisOsorio]

I'm excited to test this out