Introduce AWS Security Mapping #21622

imjalpreet · 2024-01-02T21:10:29Z

Description

This will allow flexible security mapping for AWS Lake Formation(and AWS Glue) and AWS S3 API calls, allowing for separate IAM roles or IAM credentials for specific users.

Motivation and Context

#20851

Impact

This PR introduces the ability to use multiple IAM roles / IAM credentials for different Presto Users to access AWS S3.
Follow-up PRs will allow the same for AWS Lake Formation.

Test Plan

Tested with different IAM Roles for different Presto Users when accessing AWS S3 using the Hive Connector

Contributor checklist

Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Hive Changes
* Introduce AWS Security Mapping which will allow flexible mapping of Presto Users to AWS Credentials or IAM Roles for different AWS Services

imjalpreet · 2024-01-25T05:49:55Z

@tdcmeehan can you please help review this when you get a chance? Thanks

steveburnett · 2024-01-26T18:43:50Z

When you can, please add documentation for how users can specify IAM roles or IAM credentials. If I can help, let me know!

imjalpreet · 2024-01-30T20:42:29Z

@steveburnett Yes sure, I have a draft in my local, I will refine it and push it as well for your review.

tdcmeehan · 2024-03-18T13:57:54Z

...-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingsSupplier.java

+    private static final Logger log = Logger.get(AWSSecurityMappingsSupplier.class);
+    private final Supplier<AWSSecurityMappings> mappingsSupplier;
+
+    public AWSSecurityMappingsSupplier(Optional<File> configFile, Optional<Duration> refreshPeriod)


Validate inputs

You might want to check the file exists here

I have added the check in the getMappings method

tdcmeehan · 2024-03-18T13:58:36Z

...-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingsSupplier.java

+        }
+
+        Supplier<AWSSecurityMappings> supplier = () -> parseJson(configFile.get().toPath(), AWSSecurityMappings.class);
+        return refreshPeriod.<Supplier<AWSSecurityMappings>>map(duration -> Suppliers.memoizeWithExpiration(


Please clean up formatting. orElseGet in new line, appropriately indented.

github-actions · 2024-04-02T21:13:17Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 39e11a5...d23c401.

No notifications.

imjalpreet · 2024-04-02T21:14:49Z

@tdcmeehan sorry for the delay, please let me know in case you have more feedback. I have made the updates based on your previous review comments.

@steveburnett I have pushed the documentation as you had requested. Please let me know if you would like any changes.

steveburnett · 2024-04-02T22:21:55Z

@imjalpreet, when I try to build the doc from the branch in this PR I get the following error:

"python3 -m sphinx -b html -n -d target/doctrees -j auto src/main/sphinx target/html
Running Sphinx v4.4.0

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
make: *** [html] Error 2"

I think this may be because this PR predates PR 21708.

Would you update this branch to pick up the change in PR 21708? I'm happy to try again after that.

imjalpreet · 2024-04-02T22:26:31Z

@steveburnett Thanks, I have rebased the branch on latest master.

steveburnett · 2024-04-02T22:41:21Z

@steveburnett Thanks, I have rebased the branch on latest master.

Thanks! A local doc build is successful now and I see your new docs in the result.

steveburnett

Thanks for the documentation! It makes sense to me. I commented on a few nits, and suggested moving some of the text to separate the s3 and lakeformation information as their own sections. Let me know what you think, and if any of my suggestions are unclear.

presto-docs/src/main/sphinx/connector/hive.rst

tdcmeehan

Overall looks good, just waiting for earlier comments to be addressed.

imjalpreet · 2024-04-24T05:27:22Z

@steveburnett Thank you for the review and suggestions, I have made the changes.

@tdcmeehan sorry for the delay, the review comments have been addressed.

steveburnett

LGTM! (docs)

Pull updated branch, new local build of docs, everything looks good. Thanks!

imjalpreet · 2024-05-06T07:37:53Z

@tdcmeehan just a reminder, all the comments were addressed.

elharo

This all looks to me like it belongs to a separate module, not presto-hive-common

elharo · 2024-05-21T17:51:23Z

presto-hive-common/pom.xml

@@ -95,6 +105,11 @@
            <artifactId>drift-api</artifactId>
        </dependency>

+        <dependency>


I'd prefer not to add this dependency to common code. Anyway it can be isolated to just the aws package?

I just realised that I am only using one class from this dependency in the class com.facebook.presto.hive.aws.security.s3.AWSS3SecurityMapping.

It is being used to form the credentials object here: https://github.com/prestodb/presto/pull/21622/files#diff-33e654b04e220ef6b736fee85d4cf10f15e0d5255c6af614ce8d4a2000db2b27R51.

We have a couple of options on how we can exclude the aws dependency from hive-common:

Make a clone(or similar class) of the BasicAWSCredentials class within Presto and use that here

Rather than creating the credentials object here, modify the current implementation and keep the access key and secret key as is in this class and, create the credentials object when required.

CC: @tdcmeehan

Option 1 sounds fine to me, because we just need the key and secret key, and those can just be opaque strings.

elharo · 2024-05-21T17:52:49Z

presto-hive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappings.java

+                "No AWS Security mappings configured");
+        checkArgument(
+                (awsLakeFormationSecurityMappings == null) || (awsS3SecurityMappings == null),
+                "AWS S3 Security Mapping is not allowed if AWS Lake Formation Security Mapping is configured");


and vice versa

elharo · 2024-05-21T17:53:23Z

...-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingsSupplier.java

+
+        Supplier<AWSSecurityMappings> supplier = () -> parseJson(configFile.get().toPath(), AWSSecurityMappings.class);
+        return refreshPeriod.<Supplier<AWSSecurityMappings>>map(duration -> Suppliers.memoizeWithExpiration(
+                () -> {


log message not really needed; level debug at most

tdcmeehan · 2024-05-21T19:51:04Z

This all looks to me like it belongs to a separate module, not presto-hive-common

@imjalpreet what do you think about this?

I know that Lake Formation also supports Iceberg tables. Would that mean it would need to be placed in a common module?

imjalpreet · 2024-05-22T12:44:45Z

This all looks to me like it belongs to a separate module, not presto-hive-common

what do you think about this?

I know that Lake Formation also supports Iceberg tables. Would that mean it would need to be placed in a common module?

@elharo @tdcmeehan The reason I had to include these classes in hive-common as these classes would be used from both presto-hive and presto-hive-metastore. What would be the ideal package that you think these should be included in? I can try to see if that would be possible.

imjalpreet · 2024-05-22T23:26:13Z

@tdcmeehan @elharo I have updated the PR based on the discussion and review comments. Please let me know if there are any additional comments.

elharo · 2024-05-23T11:36:06Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+    public Optional<File> getConfigFile()
+    {
+        if (configFile != null) {
+            checkArgument(configFile.exists() && configFile.isFile(), "AWS Lake Formation Security Mapping config file does not exist: %s", configFile);


configFile is not an argument so this should be an IllegalStateException or an IOException, not an IllegalArgumentException. I'm tempted to say we shouldn't check for existence here at all.

I have moved this check to where the file is getting used as you suggested

elharo · 2024-05-23T11:37:20Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+    @ConfigDescription("JSON configuration file containing AWS IAM Security mappings")
+    public AWSSecurityMappingConfig setConfigFile(File configFile)
+    {
+        this.configFile = configFile;


Yes, definitely don't check in getConfig. Possibly check when the object is created or when the file is read, but not in the getter method.

elharo · 2024-05-23T11:37:40Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+
+    @Config("hive.aws.security-mapping.config-file")
+    @ConfigDescription("JSON configuration file containing AWS IAM Security mappings")
+    public AWSSecurityMappingConfig setConfigFile(File configFile)


Do we explicitly mark things nullable? This is nullable.

Added the annotation

elharo · 2024-05-23T11:38:16Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+    @MinDuration("0ms")
+    @Config("hive.aws.security-mapping.refresh-period")
+    @ConfigDescription("Time interval after which AWS IAM security mapping configuration will be refreshed")
+    public AWSSecurityMappingConfig setRefreshPeriod(Duration refreshPeriod)


do you want this to be nullable? It is.

I'm a little surprised the object is mutable.

do you want this to be nullable? It is.

I don't think this will ever be null since null is not an acceptable value for Duration. Do you see any issue?

I'm a little surprised the object is mutable.

Since new users can be added or credentials for existing users might be modified at any time. To make that change dynamic without a cluster restart, I have made this object mutable.

All someone has to do to make this null is pass null to this method. If that's nopt OK, put a requireNonNull here. If it is OK, annotate it as nullable.

elharo · 2024-05-23T11:40:07Z

...-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingsSupplier.java

+    private static final Logger log = Logger.get(AWSSecurityMappingsSupplier.class);
+    private final Supplier<AWSSecurityMappings> mappingsSupplier;
+
+    public AWSSecurityMappingsSupplier(Optional<File> configFile, Optional<Duration> refreshPeriod)


You might want to check the file exists here

elharo · 2024-05-23T11:44:14Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/security/TestAWSS3SecurityMapping.java

+
+        applyMapping(provider, selector, configuration);
+
+        assertEquals(configuration.get(S3_ACCESS_KEY), mappingResult.getAccessKey().orElse(null));


orElse in an assert feels strange to me. Just use get(). If it's not present the assert will fail

Modified this

elharo · 2024-05-23T11:45:18Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/security/TestAWSS3SecurityMapping.java

+import static com.facebook.presto.hive.s3.security.TestAWSS3SecurityMapping.MappingSelector.empty;
+import static com.google.common.io.Resources.getResource;
+import static java.util.Objects.requireNonNull;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;


Personal preference, but I try to stick to a single test framework for simplicity. Mixing and matching is confusing

elharo · 2024-05-23T11:46:13Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/security/TestAWSS3SecurityMapping.java

+    {
+        Configuration configuration = new Configuration(false);
+
+        assertThatThrownBy(() -> applyMapping(provider, selector, configuration))


use the expectedExceptions parameter in the @Test annotation instead

Moved to exceptedExceptions.

There are many classes in Presto that are using assertThatThrownBy along with TestNG. We can plan the refactor later.

elharo · 2024-05-23T11:46:44Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/security/TestAWSS3SecurityMapping.java

+        }
+    }
+
+    protected static class MappingResult


elharo · 2024-05-23T11:47:32Z

presto-hive/src/test/java/com/facebook/presto/hive/s3/security/TestAWSS3SecurityMapping.java

+
+        private MappingResult(Optional<String> accessKey, Optional<String> secretKey, Optional<String> role)
+        {
+            this.accessKey = requireNonNull(accessKey, "accessKey is null");


requireNonNull really isn't needed in private test code

imjalpreet · 2024-05-28T10:09:11Z

@elharo I have added some changes and comments above, please let me know in case you have more comments.

elharo · 2024-05-28T14:45:28Z

presto-docs/src/main/sphinx/connector/hive.rst

+AWS Security Mapping
+^^^^^^^^^^^^^^^^^^^^
+
+Presto supports flexible security mapping for AWS Lake Formation, AWS Glue, and AWS S3 API calls, allowing for separate


Is "security mapping" defined somewhere else? This is not a term I've heard before.

It's not a generic term, we named it like that since it maps different Presto User Identities to AWS Credentials. Let me know if you have any ideas on the naming.

I think you just need to say that in the docs and the code then.

The current explanation here on line 389-390: "... allowing for separate credentials or IAM roles for specific users." do you think we should rephrase this better? We tried to explain what we mean by security mapping here.

There are more details on what each mapping should include in the following lines

But you still never say what a security mapping is. You need to define that. Easiest way might be to rewrite the first paragraph without using the phrase "security mapping".

Sure, I will rephrase it.

elharo · 2024-05-28T14:48:13Z

presto-hive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappings.java

+            @JsonProperty("lakeformation") List<AWSLakeFormationSecurityMapping> awsLakeFormationSecurityMappings,
+            @JsonProperty("s3") List<AWSS3SecurityMapping> awsS3SecurityMappings)
+    {
+        checkArgument(


This is awkward. Anyway there could be two constructors or two classes to handle the two different cases?

In my initial design, I had both of them(AWS S3 and AWS Lake Formation) as separate features, which meant both had their own set of configs to enable the feature. I felt it would be similar code for both the features and since they are a subset of AWS services, we could have a common feature that can be used either for S3 or Lake Formation depending upon the use-case.

Since only one constructor can be annotated with @JsonCreator, we had to do it this way since we use this class to parse the Json File which has the mappings.

elharo · 2024-05-28T14:49:12Z

presto-hive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappings.java

+                ImmutableList.of();
+    }
+
+    public boolean isAWSLakeFormationSecurityMappingEnabled()


These is methods really make me think adding some subclasses would clean this up.

Just thinking out loud, if I add a new parameter in the json file at the beginning something like service which can have two values s3 or lakeformation. Instead of the "is" methods, I can introduce the getServiceName method based on which the relevant feature would be enabled, for eg. in AWSS3SecurityMappingConfigurationProvider#updateConfiguration I will enable it only if service is S3.

Do you think this would be more readable/cleaner?

done

imjalpreet · 2024-05-29T13:14:20Z

@tdcmeehan Can you please have another look in case you have any other comments?

elharo · 2024-05-29T14:08:09Z

presto-docs/src/main/sphinx/connector/hive.rst

+AWS Security Mapping
+^^^^^^^^^^^^^^^^^^^^
+
+Presto supports flexible security mapping for AWS Lake Formation, AWS Glue, and AWS S3 API calls, allowing for separate


But you still never say what a security mapping is. You need to define that. Easiest way might be to rewrite the first paragraph without using the phrase "security mapping".

elharo · 2024-05-29T14:08:30Z

presto-docs/src/main/sphinx/connector/hive.rst

+A security mapping entry can be of two types: ``s3`` or ``lakeformation``.
+
+The security mapping entries are processed in the order listed in the configuration
+file. More specific mappings should thus be specified before less specific mappings.


delete "thus"

Unaddressed

Updated it, somehow missed it earlier.

elharo · 2024-05-29T14:11:37Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+    @MinDuration("0ms")
+    @Config("hive.aws.security-mapping.refresh-period")
+    @ConfigDescription("Time interval after which AWS IAM security mapping configuration will be refreshed")
+    public AWSSecurityMappingConfig setRefreshPeriod(Duration refreshPeriod)


All someone has to do to make this null is pass null to this method. If that's nopt OK, put a requireNonNull here. If it is OK, annotate it as nullable.

elharo · 2024-05-29T14:19:59Z

...ava/com/facebook/presto/hive/aws/security/lakeformation/AWSLakeFormationSecurityMapping.java

+import static com.google.common.base.MoreObjects.toStringHelper;
+import static java.util.Objects.requireNonNull;
+
+public class AWSLakeFormationSecurityMapping


It looks like AWSLakeFormationSecurityMapping and AWSS3SecurityMapping should have a common, possibly abstract, SecurityMapping superclass. That might allow you to avoid some of the problems in AWSSecurityMappings which could work exclusively with the supertype.

Sure, let me see what is the best way to simplify the implementation.

imjalpreet · 2024-05-31T17:38:12Z

@elharo I have refactored the previous implementation to try to address the points you mentioned, can you please have a look and let me know if this looks better? I have kept a separate commit for now.

elharo · 2024-05-31T17:42:28Z

...ive-common/src/main/java/com/facebook/presto/hive/aws/security/AWSSecurityMappingConfig.java

+        return this;
+    }
+
+    @AssertTrue(message = "MAPPING TYPE(" + MAPPING_TYPE + ") must be configured when AWS Security Mapping Config File(" + CONFIG_FILE + ") is set and vice versa")


I haven't seen this annotation before. Where is this coming from?

This is part of the below dependency:

<dependency> <groupId>javax.validation</groupId> <artifactId>validation-api</artifactId> </dependency>

This is being used in a few more places already in Presto like com.facebook.presto.server.InternalCommunicationConfig#isRequiredSharedSecretSet, com.facebook.presto.sql.analyzer.FeaturesConfig#isSpillerSpillPathsConfiguredIfSpillEnabled

imjalpreet · 2024-05-31T21:15:14Z

@elharo Thank you for the review.

@tdcmeehan The PR is ready for a final review from your side.

imjalpreet · 2024-06-04T11:01:38Z

@tdcmeehan just a reminder, this PR is ready for final review.

tdcmeehan

Just a couple of nits, happy to merge once addressed

tdcmeehan · 2024-06-04T15:21:41Z

...ain/java/com/facebook/presto/hive/s3/security/AWSS3SecurityMappingConfigurationProvider.java

+
+        AWSSecurityMapping awsS3SecurityMapping = mappings.get().getAWSS3SecurityMapping(context.getIdentity().getUser());
+
+        verify(


Suggested change

verify(

checkArgument(

@tdcmeehan just wanted to confirm, I used verify since the expression I was trying to evaluate was not an argument to this method. Is it still okay to use checkArgument?

I think checkArgument is more appropriate, because it throws an illegal argument exception, which more accurately describes the problem by my read.

Sure, I have updated it to checkArgument.

This will allow flexible security mapping for AWS Lake Formation(and AWS Glue) and AWS S3 API calls, allowing for separate IAM roles or IAM credentials for specific users.

imjalpreet self-assigned this Jan 2, 2024

imjalpreet requested a review from a team as a code owner January 2, 2024 21:10

imjalpreet requested a review from presto-oss January 2, 2024 21:10

imjalpreet force-pushed the AWSSecurityMapping branch from 0b99300 to a08e98b Compare January 2, 2024 21:59

imjalpreet requested a review from tdcmeehan January 2, 2024 22:18

imjalpreet mentioned this pull request Jan 2, 2024

Presto and AWS Lake Formation Integration #20851

Open

6 tasks

tdcmeehan self-assigned this Jan 3, 2024

imjalpreet requested a review from agrawalreetika February 8, 2024 11:42

tdcmeehan reviewed Mar 18, 2024

View reviewed changes

imjalpreet force-pushed the AWSSecurityMapping branch from a08e98b to a90d3e6 Compare April 2, 2024 21:12

imjalpreet requested a review from steveburnett as a code owner April 2, 2024 21:12

imjalpreet force-pushed the AWSSecurityMapping branch from a90d3e6 to 49f93f7 Compare April 2, 2024 22:26

steveburnett requested changes Apr 3, 2024

View reviewed changes

tdcmeehan reviewed Apr 16, 2024

View reviewed changes

imjalpreet force-pushed the AWSSecurityMapping branch from 49f93f7 to 11812b8 Compare April 24, 2024 05:24

imjalpreet requested a review from steveburnett April 24, 2024 05:27

steveburnett previously approved these changes Apr 24, 2024

View reviewed changes

elharo reviewed May 21, 2024

View reviewed changes

imjalpreet dismissed steveburnett’s stale review via 19d82dc May 22, 2024 23:05

imjalpreet force-pushed the AWSSecurityMapping branch from 11812b8 to 19d82dc Compare May 22, 2024 23:05

Add option to set identity when creating TestingConnectorSession

1a615b8

imjalpreet force-pushed the AWSSecurityMapping branch from 19d82dc to 5e8bedb Compare May 22, 2024 23:20

elharo previously requested changes May 23, 2024

View reviewed changes

imjalpreet force-pushed the AWSSecurityMapping branch from 5e8bedb to 2e3fea1 Compare May 28, 2024 09:46

elharo reviewed May 28, 2024

View reviewed changes

elharo reviewed May 29, 2024

View reviewed changes

elharo reviewed May 31, 2024

View reviewed changes

elharo previously approved these changes May 31, 2024

View reviewed changes

imjalpreet force-pushed the AWSSecurityMapping branch from d728296 to 0881c39 Compare May 31, 2024 20:10

tdcmeehan reviewed Jun 4, 2024

View reviewed changes

imjalpreet added 3 commits June 5, 2024 00:48

Introduce AWS Security Mapping

e0a0a9c

This will allow flexible security mapping for AWS Lake Formation(and AWS Glue) and AWS S3 API calls, allowing for separate IAM roles or IAM credentials for specific users.

Implement DynamicConfigurationProvider for AWS S3 Security Mapping

0742ef2

Add documentation for AWS Security Mapping

d23c401

imjalpreet dismissed elharo’s stale review via d23c401 June 4, 2024 19:20

imjalpreet force-pushed the AWSSecurityMapping branch from 0881c39 to d23c401 Compare June 4, 2024 19:20

tdcmeehan approved these changes Jun 4, 2024

View reviewed changes

imjalpreet merged commit 07c1b33 into prestodb:master Jun 4, 2024
57 checks passed

wanglinsong mentioned this pull request Jun 25, 2024

Add release notes for 0.288 #23079

Merged

36 tasks


		applyMapping(provider, selector, configuration);

		assertEquals(configuration.get(S3_ACCESS_KEY), mappingResult.getAccessKey().orElse(null));


		AWSSecurityMapping awsS3SecurityMapping = mappings.get().getAWSS3SecurityMapping(context.getIdentity().getUser());

		verify(

Introduce AWS Security Mapping #21622

Introduce AWS Security Mapping #21622

Conversation

imjalpreet commented Jan 2, 2024 • edited Loading

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

imjalpreet commented Jan 25, 2024

steveburnett commented Jan 26, 2024

imjalpreet commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 2, 2024 • edited Loading

imjalpreet commented Apr 2, 2024

steveburnett commented Apr 2, 2024

imjalpreet commented Apr 2, 2024

steveburnett commented Apr 2, 2024

steveburnett left a comment

Choose a reason for hiding this comment

tdcmeehan left a comment

Choose a reason for hiding this comment

imjalpreet commented Apr 24, 2024

steveburnett left a comment

Choose a reason for hiding this comment

imjalpreet commented May 6, 2024

elharo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tdcmeehan commented May 21, 2024

imjalpreet commented May 22, 2024

imjalpreet commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjalpreet commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjalpreet commented May 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imjalpreet commented May 31, 2024

Choose a reason for hiding this comment

imjalpreet commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Apr 2, 2024 •

edited

Loading