Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SALTO-7409: Handle "INSUFFICIENT_ACCESS: insufficient access rights on entity" errors in retrieve #7232

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AlmogMesilaty
Copy link
Contributor

@AlmogMesilaty AlmogMesilaty commented Feb 10, 2025

SALTO-7409: Handle "INSUFFICIENT_ACCESS: insufficient access rights on entity" errors in retrieve


Release Notes:
None


User Notifications:
None

@coveralls
Copy link

coveralls commented Feb 10, 2025

Coverage Status

coverage: 93.634% (-0.002%) from 93.636%
when pulling 833addf on AlmogMesilaty:SALTO-7409
into 4831357 on salto-io:main.

@AlmogMesilaty AlmogMesilaty requested a review from yelly February 10, 2025 11:17
const errorPattern = /INSUFFICIENT_ACCESS: insufficient access rights on entity: (\w+)/g
const matches = [...error.message.matchAll(errorPattern)]
if (matches.length > 0) {
matches.forEach(match => {
Copy link
Contributor

Choose a reason for hiding this comment

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

How can there be multiple matches?

typesWithInsufficientAccess.add(failedEntity)
})
const updatedFilesToRetrieve = CurrentFilesToRetrieve.filter(
fileProp => ![...matches.map(match => match[1])].includes(fileProp.type),
Copy link
Contributor

Choose a reason for hiding this comment

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

This will also be simpler once you assume there's only 1 match.

fileProp => ![...matches.map(match => match[1])].includes(fileProp.type),
)
if (updatedFilesToRetrieve.length === 0) {
throw new Error('No files left to retrieve after filtering out failed entities')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this an error? Just return...

const result = await client.retrieve(request)

const typesWithInsufficientAccess = new Set<string>()
const retrieveWithRetry = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a great name because the real functionality this function provides is not the retry but rather the handling of types with insufficient access.
Maybe call it retrieveAccessibleEntities or something like that.

configChanges.push(
createSkippedListConfigChange({
type: entity,
reason: `Insufficient access rights on entity: ${entity}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be written in more Salto language by using "type" instead of "entity".

types: [mockTypes.CustomObject, mockTypes.Flow, mockTypes.Opportunity],
fetchProfile,
})
// expect(elements).toHaveLength(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks left over.

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.

3 participants