feat: wildcard support #390

jcosentino11 · 2023-08-04T04:47:26Z

Issue #, if available:

Description of changes:

Allow policy resources with wildcards, such as "mqtt:topic:my*". Matching is done via WildcardTrie.

Also refactored policy-related integration tests

Why is this change necessary:

How was this change tested:
Integration tests

Any additional information or context required to review the change:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2023-08-04T05:13:33Z

Unit Tests Coverage Report

File	Coverage	Lines	Branches
All files	`73%`	`80%`	`66%`	✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils	`79%`	`86%`	`71%`	✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Operation	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Resource	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.CertificateManager	`80%`	`90%`	`69%`	✅
com.aws.greengrass.clientdevices.auth.ClientDevicesAuthService	`79%`	`90%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.DeviceAuthClient	`74%`	`83%`	`64%`	✅
com.aws.greengrass.clientdevices.auth.certificate.ClientCertificateGenerator	`95%`	`90%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper$ProviderType	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper	`74%`	`92%`	`56%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore	`72%`	`85%`	`60%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor	`77%`	`87%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.certificate.ServerCertificateGenerator	`93%`	`87%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateGenerator	`70%`	`90%`	`50%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore$CAType	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor$CertRotationDecider	`90%`	`100%`	`80%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificatesConfig	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.session.events.SessionCreationEvent$SessionCreationStatus	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyIotCertificate	`94%`	`88%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyThingAttachedToCertificate	`89%`	`91%`	`88%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.CreateIoTThingSession	`88%`	`93%`	`83%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyCertificateValidityPeriod	`88%`	`88%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyThingAttachedToCertificate$Result$VerificationSource	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.infra.ClientCertificateStore	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.certificate.infra.BackgroundCertificateRefresh	`83%`	`85%`	`82%`	✅
com.aws.greengrass.clientdevices.auth.iot.infra.ThingRegistry	`92%`	`97%`	`88%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureManagedCertificateAuthority	`85%`	`85%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureCustomCertificateAuthority	`83%`	`83%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.RegisterCertificateAuthorityUseCase	`65%`	`81%`	`50%`	✅
com.aws.greengrass.clientdevices.auth.configuration.MetricsConfiguration	`83%`	`100%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.AuthorizationPolicyStatement$Effect	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupManager	`89%`	`94%`	`83%`	✅
com.aws.greengrass.clientdevices.auth.configuration.ConfigurationFormatVersion	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.CAConfiguration	`96%`	`100%`	`92%`	✅
com.aws.greengrass.clientdevices.auth.configuration.RuntimeConfiguration	`83%`	`96%`	`70%`	✅
com.aws.greengrass.clientdevices.auth.configuration.SecurityConfiguration	`80%`	`93%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupDefinition	`75%`	`100%`	`50%`	✅
com.aws.greengrass.clientdevices.auth.configuration.ExpressionVisitor	`84%`	`94%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupConfiguration	`90%`	`95%`	`86%`	✅
com.aws.greengrass.clientdevices.auth.api.ServiceErrorEvent	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.api.ClientDevicesAuthServiceApi	`90%`	`80%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.api.DomainEvents	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.api.AuthorizeClientDeviceActionEvent$AuthorizationStatus	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.api.UseCases	`71%`	`92%`	`50%`	✅
com.aws.greengrass.clientdevices.auth.api.DomainEvent	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.api.GetCertificateRequestOptions$CertificateType	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.session.attribute.StringLiteralAttribute	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.session.attribute.WildcardSuffixAttribute	`88%`	`100%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.certificate.events.CertificateSubscriptionEvent$SubscriptionStatus	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.events.CACertificateChainChanged	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.Certificate$Status	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.IotAuthClient$Default	`56%`	`47%`	`64%`	✅
com.aws.greengrass.clientdevices.auth.iot.Thing	`87%`	`93%`	`82%`	✅
com.aws.greengrass.clientdevices.auth.iot.Certificate	`78%`	`89%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.iot.GreengrassV2DataClientFactory	`18%`	`18%`	`0%`	❌
com.aws.greengrass.clientdevices.auth.iot.CertificateRegistry	`95%`	`90%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.iot.Component	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.events.VerifyClientDeviceIdentityEvent$VerificationStatus	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.infra.NetworkStateProvider$Default$1	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.infra.NetworkStateProvider$ConnectionState	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.infra.NetworkStateProvider$Default	`83%`	`97%`	`70%`	✅
com.aws.greengrass.ipc.IPCUtils	`83%`	`67%`	`100%`	✅
com.aws.greengrass.ipc.VerifyClientDeviceIdentityOperationHandler	`60%`	`69%`	`50%`	✅
com.aws.greengrass.ipc.GetClientDeviceAuthTokenOperationHandler	`86%`	`98%`	`75%`	✅
com.aws.greengrass.ipc.AuthorizeClientDeviceActionOperationHandler	`79%`	`92%`	`67%`	✅
com.aws.greengrass.ipc.SubscribeToCertificateUpdatesOperationHandler	`81%`	`88%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionConfig	`92%`	`100%`	`83%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionManager$1	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionCreator	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionManager	`88%`	`100%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionImpl	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionCreator$SessionFactorySingleton	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory$MqttCredential	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CACertificateChainChangedHandler	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CAConfigurationChangedHandler	`93%`	`87%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CertificateRotationHandler	`96%`	`91%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.SecurityConfigurationChangedHandler	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.SessionCreationEventHandler	`88%`	`100%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.MetricsConfigurationChangedHandler	`70%`	`90%`	`50%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.AuthorizeClientDeviceActionsMetricHandler	`88%`	`100%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.VerifyClientDeviceIdentityEventHandler	`88%`	`100%`	`75%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.CertificateSubscriptionEventHandler	`83%`	`100%`	`67%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.ServiceErrorEventHandler	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.iot.dto.CertificateV1DTO$Status	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.usecases.GetConnectivityInformationUseCase	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.usecases.RecordConnectivityChangesUseCase	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.util.ResizableLinkedBlockingQueue	`90%`	`80%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.util.ParseIPAddress	`90%`	`95%`	`84%`	✅
com.aws.greengrass.clientdevices.auth.metrics.ClientDeviceAuthMetrics	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.metrics.MetricsEmitter	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.ConnectivityInfoCache	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor	`68%`	`79%`	`57%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesResponse	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.HostAddress	`67%`	`67%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesRequest	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.ConnectivityInformation	`100%`	`100%`	`100%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionConstants	`100%`	`100%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.TokenMgrError	`22%`	`32%`	`12%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTokenManager	`61%`	`65%`	`58%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTStart	`33%`	`33%`	`0%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTAnd	`67%`	`67%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.Token	`58%`	`58%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionDefaultVisitor	`0%`	`0%`	`0%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTOr	`67%`	`67%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleCharStream	`28%`	`31%`	`25%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTreeConstants	`0%`	`0%`	`0%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.JJTRuleExpressionState	`67%`	`65%`	`70%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTThing	`67%`	`67%`	`0%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpression	`63%`	`63%`	`62%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleNode	`27%`	`35%`	`19%`	❌

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 8a8e202

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java

...eengrass/integrationtests/deviceauth/EvaluateClientDeviceActionsWithPolicyVariablesTest.java

...integrationtests/java/com/aws/greengrass/integrationtests/deviceauth/PolicyWildcardTest.java

src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java

github-actions · 2024-02-02T06:50:53Z

Code Coverage Report

File	Coverage
All files	`74%`	✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils	`90%`	✅
com.aws.greengrass.clientdevices.auth.CertificateManager	`88%`	✅
com.aws.greengrass.clientdevices.auth.ClientDevicesAuthService	`77%`	✅
com.aws.greengrass.clientdevices.auth.DeviceAuthClient	`73%`	✅
com.aws.greengrass.clientdevices.auth.certificate.ClientCertificateGenerator	`95%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper	`74%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore	`72%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor	`77%`	✅
com.aws.greengrass.clientdevices.auth.certificate.ServerCertificateGenerator	`93%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateGenerator	`70%`	✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor$CertRotationDecider	`90%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyIotCertificate	`94%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyThingAttachedToCertificate	`89%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.CreateIoTThingSession	`67%`	✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyCertificateValidityPeriod	`88%`	✅
com.aws.greengrass.clientdevices.auth.certificate.infra.BackgroundCertificateRefresh	`82%`	✅
com.aws.greengrass.clientdevices.auth.iot.infra.ThingRegistry	`92%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureManagedCertificateAuthority	`85%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureCustomCertificateAuthority	`83%`	✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.RegisterCertificateAuthorityUseCase	`65%`	✅
com.aws.greengrass.clientdevices.auth.configuration.MetricsConfiguration	`83%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupManager	`94%`	✅
com.aws.greengrass.clientdevices.auth.configuration.CAConfiguration	`96%`	✅
com.aws.greengrass.clientdevices.auth.configuration.RuntimeConfiguration	`83%`	✅
com.aws.greengrass.clientdevices.auth.configuration.SecurityConfiguration	`80%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupDefinition	`75%`	✅
com.aws.greengrass.clientdevices.auth.configuration.ExpressionVisitor	`84%`	✅
com.aws.greengrass.clientdevices.auth.configuration.GroupConfiguration	`92%`	✅
com.aws.greengrass.clientdevices.auth.api.ClientDevicesAuthServiceApi	`85%`	✅
com.aws.greengrass.clientdevices.auth.api.GetCertificateRequestWithGenerator	`75%`	✅
com.aws.greengrass.clientdevices.auth.api.UseCases	`71%`	✅
com.aws.greengrass.clientdevices.auth.session.attribute.WildcardSuffixAttribute	`96%`	✅
com.aws.greengrass.clientdevices.auth.iot.IotAuthClient$Default	`56%`	✅
com.aws.greengrass.clientdevices.auth.iot.Thing	`85%`	✅
com.aws.greengrass.clientdevices.auth.iot.Certificate	`74%`	✅
com.aws.greengrass.clientdevices.auth.iot.GreengrassV2DataClientFactory	`18%`	❌
com.aws.greengrass.clientdevices.auth.iot.CertificateRegistry	`95%`	✅
com.aws.greengrass.clientdevices.auth.infra.NetworkStateProvider$Default	`83%`	✅
com.aws.greengrass.ipc.IPCUtils	`83%`	✅
com.aws.greengrass.ipc.VerifyClientDeviceIdentityOperationHandler	`60%`	✅
com.aws.greengrass.ipc.GetClientDeviceAuthTokenOperationHandler	`83%`	✅
com.aws.greengrass.ipc.AuthorizeClientDeviceActionOperationHandler	`79%`	✅
com.aws.greengrass.ipc.SubscribeToCertificateUpdatesOperationHandler	`81%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionConfig	`92%`	✅
com.aws.greengrass.clientdevices.auth.session.SessionManager	`88%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CAConfigurationChangedHandler	`93%`	✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CertificateRotationHandler	`96%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.SessionCreationEventHandler	`88%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.MetricsConfigurationChangedHandler	`70%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.AuthorizeClientDeviceActionsMetricHandler	`88%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.VerifyClientDeviceIdentityEventHandler	`88%`	✅
com.aws.greengrass.clientdevices.auth.metrics.handlers.CertificateSubscriptionEventHandler	`83%`	✅
com.aws.greengrass.clientdevices.auth.util.ResizableLinkedBlockingQueue	`90%`	✅
com.aws.greengrass.clientdevices.auth.util.ParseIPAddress	`90%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor$SucceedOnceOperation	`88%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor	`79%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor$ProcessCISShadowTask	`76%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor$CISShadowTaskQueue	`88%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.HostAddress	`67%`	✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor$CISShadowTaskExecutor	`80%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.TokenMgrError	`22%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTokenManager	`61%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTStart	`33%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTAnd	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.Token	`58%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionDefaultVisitor	`0%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTOr	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleCharStream	`28%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTreeConstants	`0%`	❌
com.aws.greengrass.clientdevices.auth.configuration.parser.JJTRuleExpressionState	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTThing	`67%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpression	`63%`	✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleNode	`27%`	❌

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 50fc7c1

github-actions · 2024-02-02T07:07:57Z

Benchmark Results

Benchmark	Score
com.aws.greengrass.clientdevices.auth.benchmark.AuthorizationBenchmarks.GIVEN_policy_with_thing_name_variable_WHEN_auth_request_THEN_successful_auth	1286219.87 ops/s
com.aws.greengrass.clientdevices.auth.benchmark.AuthorizationBenchmarks.GIVEN_policy_with_wildcards_WHEN_auth_request_THEN_successful_auth	208251.16 ops/s
com.aws.greengrass.clientdevices.auth.benchmark.AuthorizationBenchmarks.GIVEN_single_group_permission_WHEN_simple_auth_request_THEN_successful_auth	2458760.53 ops/s

MikeDombo · 2024-02-02T12:35:54Z

Can you do a profile and see if there's anything you can do about the performance? This is significantly slower than the non wildcard tests according to the benchmark.

MikeDombo · 2024-02-02T12:36:41Z

Does this change work with the policy variables too, can you mix both?

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java

jcosentino11 · 2024-02-02T17:01:44Z

Does this change work with the policy variables too, can you mix both?

yep there's an integ test for it. would like to add more coverage, but wanted to do some refactoring of existing unit tests first in separate change

jcosentino11 · 2024-02-02T17:02:11Z

Can you do a profile and see if there's anything you can do about the performance? This is significantly slower than the non wildcard tests according to the benchmark.

noticed that trie is using a synchronized map, can explore if using non synchronized would have significant impact

MikeDombo · 2024-02-02T17:35:49Z

Can you do a profile and see if there's anything you can do about the performance? This is significantly slower than the non wildcard tests according to the benchmark.

noticed that trie is using a synchronized map, can explore if using non synchronized would have significant impact

Where do you see any synchronized? WildcardTrie uses a concurrent hashmap which is nearly lock free.

jcosentino11 · 2024-02-02T17:39:26Z

Can you do a profile and see if there's anything you can do about the performance? This is significantly slower than the non wildcard tests according to the benchmark.

noticed that trie is using a synchronized map, can explore if using non synchronized would have significant impact

ah, intellij profile shows constructino of WildcardTrie is what's eating up the time, initializing children map

jcosentino11 · 2024-02-02T17:40:07Z

Can you do a profile and see if there's anything you can do about the performance? This is significantly slower than the non wildcard tests according to the benchmark.

noticed that trie is using a synchronized map, can explore if using non synchronized would have significant impact

Where do you see any synchronized? WildcardTrie uses a concurrent hashmap which is nearly lock free.

saw concurrent and assumed locks 😓

src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java

jcosentino11 · 2024-02-02T18:08:49Z

other hotspots include

parseOperation() (which we already knew about because regex)
groupManager.getApplicablePolicyPermissions()

jcosentino11 · 2024-02-02T18:18:05Z

And marking down previous ops/s before comment gets overwritten: 174675

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java

src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java

MikeDombo reviewed Aug 16, 2023

View reviewed changes

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java Outdated Show resolved Hide resolved

sameerzuberi force-pushed the wildcard-support branch from cec82af to b376566 Compare January 4, 2024 23:17

jcosentino11 commented Jan 5, 2024

View reviewed changes

...eengrass/integrationtests/deviceauth/EvaluateClientDeviceActionsWithPolicyVariablesTest.java Outdated Show resolved Hide resolved

...integrationtests/java/com/aws/greengrass/integrationtests/deviceauth/PolicyWildcardTest.java Outdated Show resolved Hide resolved

Base automatically changed from policy-variable-support to main February 1, 2024 21:08

jcosentino11 force-pushed the wildcard-support branch from d4d920a to 611f7c9 Compare February 2, 2024 06:39

github-advanced-security bot found potential problems Feb 2, 2024

View reviewed changes

src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java Dismissed Show dismissed Hide dismissed

jcosentino11 requested review from MikeDombo, sameerzuberi and robcmann February 2, 2024 06:59

MikeDombo reviewed Feb 2, 2024

View reviewed changes

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java Outdated Show resolved Hide resolved

MikeDombo reviewed Feb 2, 2024

View reviewed changes

robcmann previously approved these changes Feb 2, 2024

View reviewed changes

MikeDombo reviewed Feb 2, 2024

View reviewed changes

src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java Outdated Show resolved Hide resolved

MikeDombo reviewed Feb 2, 2024

View reviewed changes

src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java Outdated Show resolved Hide resolved

jcosentino11 dismissed robcmann’s stale review via 283fc94 February 2, 2024 20:24

jcosentino11 and others added 3 commits February 6, 2024 20:51

feat: add wildcard policy support

9c62c5c

chore: pmd

70b989c

fix: use wildcard trie

8c3e35e

sameerzuberi and others added 13 commits February 6, 2024 20:52

chore: remove log

0787f69

fix: reset wildcard map for testing

c97595d

chore: fix comments

3e5ef88

fix: clear wildcard map between tests

b8f5945

chore: remove telemetry log

17793b1

chore: parameterize test

c688d16

refactor: integ tests

c5f7f0a

feat: wildcard matching

6e10576

feat: wildcard benchmark

f201233

perf: construct trie once

c7008d9

chore: properly reset trie

f9af9b7

chore: revert back to nucleus trie

a72e77e

chore: merge and add tests

11cefdc

jcosentino11 force-pushed the wildcard-support branch from af4ddfe to 11cefdc Compare February 7, 2024 06:11

jcosentino11 requested review from MikeDombo and robcmann February 7, 2024 06:29

MikeDombo previously approved these changes Feb 7, 2024

View reviewed changes

Merge branch 'main' into wildcard-support

6bf4558

robcmann previously approved these changes Feb 7, 2024

View reviewed changes

chore: test fix, include unknown variable check

50fc7c1

jcosentino11 dismissed stale reviews from robcmann and MikeDombo via 50fc7c1 February 7, 2024 19:17

jcosentino11 requested review from robcmann and MikeDombo February 7, 2024 19:18

robcmann approved these changes Feb 7, 2024

View reviewed changes

MikeDombo approved these changes Feb 7, 2024

View reviewed changes

jcosentino11 merged commit 8dd531b into main Feb 7, 2024
5 checks passed

jcosentino11 deleted the wildcard-support branch February 7, 2024 19:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: wildcard support #390

feat: wildcard support #390

jcosentino11 commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Feb 2, 2024 •

edited

Loading

github-actions bot commented Feb 2, 2024 •

edited

Loading

MikeDombo commented Feb 2, 2024

MikeDombo commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

MikeDombo commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

feat: wildcard support #390

feat: wildcard support #390

Conversation

jcosentino11 commented Aug 4, 2023 • edited Loading

github-actions bot commented Aug 4, 2023 • edited Loading

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

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

Benchmark Results

MikeDombo commented Feb 2, 2024

MikeDombo commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

MikeDombo commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Feb 2, 2024

jcosentino11 commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Feb 2, 2024 •

edited

Loading

github-actions bot commented Feb 2, 2024 •

edited

Loading