Skip to content

Commit e5d69c8

Browse files
rlirli
authored
Improve handling of disk cache errors (#3625)
* Improve handling of disk cache errors * better exception message * tst * log * tst * log * debug * assert * tst * why * tst * chown * buildspec * Update toolkit-test-log.properties * Update CredentialWriter.kt * Update DiskCache.kt * perms * tst * log * perms * faster perms * more creds
1 parent 06d1216 commit e5d69c8

File tree

7 files changed

+347
-60
lines changed

7 files changed

+347
-60
lines changed

‎.changes/next-release/bugfix-d0f3ffcc-004e-49f9-9d91-e910d4e6303e.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type" : "bugfix",
3+
"description" : "Improve handling of disk errors related to SSO and align folder permissions with AWS CLI"
4+
}

‎buildspec/linuxTests.yml‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ env:
1313
phases:
1414
install:
1515
commands:
16-
- export PATH="$PATH:$HOME/.dotnet/tools"
17-
- dotnet codeartifact-creds install
16+
- useradd codebuild-user
17+
- dnf install -y acl
18+
- chown -R codebuild-user:codebuild-user /codebuild/output
19+
- setfacl -m d:o::rwx,o::rwx /root
1820

1921
build:
2022
commands:
@@ -23,10 +25,11 @@ phases:
2325
CODEARTIFACT_URL=$(aws codeartifact get-repository-endpoint --domain $CODEARTIFACT_DOMAIN_NAME --repository $CODEARTIFACT_REPO_NAME --format maven --query repositoryEndpoint --output text)
2426
CODEARTIFACT_NUGET_URL=$(aws codeartifact get-repository-endpoint --domain $CODEARTIFACT_DOMAIN_NAME --repository $CODEARTIFACT_REPO_NAME --format nuget --query repositoryEndpoint --output text)
2527
CODEARTIFACT_AUTH_TOKEN=$(aws codeartifact get-authorization-token --domain $CODEARTIFACT_DOMAIN_NAME --query authorizationToken --output text --duration-seconds 3600)
28+
su codebuild-user -c "dotnet codeartifact-creds install"
2629
fi
2730
2831
- chmod +x gradlew
29-
- ./gradlew -PideProfileName=$ALTERNATIVE_IDE_PROFILE_NAME check coverageReport --info --console plain --continue
32+
- su codebuild-user -c "./gradlew -PideProfileName=$ALTERNATIVE_IDE_PROFILE_NAME check coverageReport --info --console plain --continue"
3033
- ./gradlew -PideProfileName=$ALTERNATIVE_IDE_PROFILE_NAME buildPlugin
3134
- VCS_COMMIT_ID="${CODEBUILD_RESOLVED_SOURCE_VERSION}"
3235
- CI_BUILD_URL=$(echo $CODEBUILD_BUILD_URL | sed 's/#/%23/g') # Encode `#` in the URL because otherwise the url is clipped in the Codecov.io site
@@ -44,7 +47,7 @@ phases:
4447
- rsync -rmq --include='*/' --include '**/build/idea-sandbox/system*/log/**' --exclude='*' . $TEST_ARTIFACTS/ || true
4548
- rsync -rmq --include='*/' --include '**/build/reports/**' --exclude='*' . $TEST_ARTIFACTS/ || true
4649
- rsync -rmq --include='*/' --include '**/test-results/**/*.xml' --exclude='*' . $TEST_ARTIFACTS/test-reports || true
47-
- cp -r ./intellij/build/distributions/*.zip $BUILD_ARTIFACTS/ || true
50+
- cp -r ./intellij/build/distributions/*.zip $BUILD_ARTIFACTS/ || touch $BUILD_ARTIFACTS/build_failed
4851

4952
reports:
5053
unit-test:

‎core/src/software/aws/toolkits/core/utils/PathUtils.kt‎

Lines changed: 175 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,26 @@
33

44
package software.aws.toolkits.core.utils
55

6+
import org.slf4j.Logger
67
import java.io.InputStream
78
import java.io.OutputStream
89
import java.nio.charset.Charset
10+
import java.nio.file.AccessMode
911
import java.nio.file.FileAlreadyExistsException
1012
import java.nio.file.Files
1113
import java.nio.file.NoSuchFileException
1214
import java.nio.file.Path
15+
import java.nio.file.attribute.AclEntry
16+
import java.nio.file.attribute.AclFileAttributeView
1317
import java.nio.file.attribute.FileTime
1418
import java.nio.file.attribute.PosixFilePermission
19+
import java.nio.file.attribute.PosixFilePermissions
20+
import java.nio.file.attribute.UserPrincipal
21+
import kotlin.io.path.getPosixFilePermissions
22+
import kotlin.io.path.isRegularFile
23+
24+
val POSIX_OWNER_ONLY_FILE = setOf(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)
25+
val POSIX_OWNER_ONLY_DIR = setOf(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE)
1526

1627
fun Path.inputStream(): InputStream = Files.newInputStream(this)
1728
fun Path.inputStreamIfExists(): InputStream? = try {
@@ -20,26 +31,183 @@ fun Path.inputStreamIfExists(): InputStream? = try {
2031
null
2132
}
2233

23-
fun Path.touch() {
24-
this.createParentDirectories()
34+
fun Path.touch(restrictToOwner: Boolean = false) {
2535
try {
26-
Files.createFile(this)
27-
} catch (_: FileAlreadyExistsException) { }
36+
if (!restrictToOwner || !hasPosixFilePermissions()) {
37+
Files.createFile(this)
38+
} else {
39+
Files.createFile(this, PosixFilePermissions.asFileAttribute(POSIX_OWNER_ONLY_FILE))
40+
}
41+
} catch (_: FileAlreadyExistsException) {}
2842
}
2943

3044
fun Path.outputStream(): OutputStream {
3145
this.createParentDirectories()
3246
return Files.newOutputStream(this)
3347
}
34-
fun Path.createParentDirectories() = Files.createDirectories(this.parent)
48+
49+
fun Path.createParentDirectories(restrictToOwner: Boolean = false) = if (!restrictToOwner || !hasPosixFilePermissions()) {
50+
Files.createDirectories(this.parent)
51+
} else {
52+
Files.createDirectories(this.parent, PosixFilePermissions.asFileAttribute(POSIX_OWNER_ONLY_DIR))
53+
}
54+
3555
fun Path.exists() = Files.exists(this)
3656
fun Path.deleteIfExists() = Files.deleteIfExists(this)
3757
fun Path.lastModified(): FileTime = Files.getLastModifiedTime(this)
3858
fun Path.readText(charset: Charset = Charsets.UTF_8) = toFile().readText(charset)
3959
fun Path.writeText(text: String, charset: Charset = Charsets.UTF_8) = toFile().writeText(text, charset)
60+
61+
// Comes from PosixFileAttributeView#name()
62+
fun Path.hasPosixFilePermissions() = "posix" in this.fileSystem.supportedFileAttributeViews()
4063
fun Path.filePermissions(permissions: Set<PosixFilePermission>) {
41-
// Comes from PosixFileAttributeView#name()
42-
if ("posix" in this.fileSystem.supportedFileAttributeViews()) {
64+
if (hasPosixFilePermissions()) {
4365
Files.setPosixFilePermissions(this, permissions)
4466
}
4567
}
68+
69+
fun Path.tryDirOp(log: Logger, block: Path.() -> Unit) {
70+
try {
71+
log.debug { "dir op on $this" }
72+
block(this)
73+
} catch (e: Exception) {
74+
if (e !is java.nio.file.AccessDeniedException && e !is kotlin.io.AccessDeniedException) {
75+
throw e
76+
}
77+
78+
if (!hasPosixFilePermissions()) {
79+
throw tryAugmentExceptionMessage(e, this)
80+
}
81+
82+
log.info(e) { "Attempting to handle ADE for directory operation" }
83+
try {
84+
var parent = if (this.isRegularFile()) { parent } else { this }
85+
86+
while (parent != null) {
87+
if (!parent.exists()) {
88+
log.info { "${parent.toAbsolutePath()}: does not exist yet" }
89+
} else {
90+
if (tryOrNull { parent.fileSystem.provider().checkAccess(parent, AccessMode.READ, AccessMode.WRITE, AccessMode.EXECUTE) } != null) {
91+
log.debug { "$parent has rwx, exiting" }
92+
// can assume parent permissions are correct
93+
break
94+
}
95+
96+
log.debug { "fixing perms for $parent" }
97+
parent.tryFixPerms(log, POSIX_OWNER_ONLY_DIR)
98+
}
99+
100+
parent = parent.parent
101+
}
102+
} catch (e2: Exception) {
103+
log.warn(e2) { "Encountered error while handling ADE for ${e.message}" }
104+
105+
throw tryAugmentExceptionMessage(e, this)
106+
}
107+
108+
log.info { "Done attempting to handle ADE for directory operation" }
109+
block(this)
110+
}
111+
}
112+
113+
fun<T> Path.tryFileOp(log: Logger, block: Path.() -> T) =
114+
try {
115+
log.debug { "file op on $this" }
116+
block(this)
117+
} catch (e: Exception) {
118+
if (e !is java.nio.file.AccessDeniedException && e !is kotlin.io.AccessDeniedException) {
119+
throw e
120+
}
121+
122+
if (!hasPosixFilePermissions()) {
123+
throw tryAugmentExceptionMessage(e, this)
124+
}
125+
126+
log.info(e) { "Attempting to handle ADE for file operation" }
127+
try {
128+
log.debug { "fixing perms for $this" }
129+
tryFixPerms(log, POSIX_OWNER_ONLY_FILE)
130+
} catch (e2: Exception) {
131+
log.warn(e2) { "Encountered error while handling ADE for ${e.message}" }
132+
133+
throw tryAugmentExceptionMessage(e, this)
134+
}
135+
136+
log.info { "Done attempting to handle ADE for file operation" }
137+
block(this)
138+
}
139+
140+
private fun Path.tryFixPerms(log: Logger, desiredPermissions: Set<PosixFilePermission>) {
141+
// TODO: consider handling linux ACLs
142+
// only try ops if we own the file
143+
// (ab)use invariant that chmod only works if you are root or the file owner
144+
val perms = tryOrLogShortException(log) { Files.getPosixFilePermissions(this) }
145+
val ownership = tryOrLogShortException(log) { Files.getOwner(this) }
146+
147+
log.info { "Permissions for ${toAbsolutePath()}: $ownership, $perms" }
148+
if (perms != null && ownership != null) {
149+
if (ownership.name != "root" && tryOrNull { filePermissions(perms) } != null) {
150+
val permissions = perms + desiredPermissions
151+
log.info { "Setting perms for ${toAbsolutePath()}: $permissions" }
152+
filePermissions(permissions)
153+
}
154+
}
155+
}
156+
157+
private fun tryAugmentExceptionMessage(e: Exception, path: Path): Exception {
158+
if (e !is java.nio.file.AccessDeniedException && e !is kotlin.io.AccessDeniedException) {
159+
return e
160+
}
161+
162+
var potentialProblem = if (path.exists()) { path } else { path.parent }
163+
var acls: List<AclEntry>? = null
164+
var ownership: UserPrincipal? = null
165+
while (potentialProblem != null) {
166+
acls = tryOrNull { Files.getFileAttributeView(potentialProblem, AclFileAttributeView::class.java).acl }
167+
ownership = tryOrNull { Files.getOwner(potentialProblem) }
168+
169+
if (acls != null || ownership != null) {
170+
break
171+
}
172+
173+
potentialProblem = potentialProblem.parent
174+
}
175+
176+
val message = buildString {
177+
// $path is automatically added to the front of the exception message
178+
appendLine("Exception trying to perform operation")
179+
180+
if (potentialProblem != null) {
181+
append("Potential issue is with $potentialProblem")
182+
183+
if (ownership != null) {
184+
append(", which has owner: $ownership")
185+
}
186+
187+
if (acls != null) {
188+
append(", and ACL entries for: ${acls.map { it.principal() }}")
189+
}
190+
191+
val posixPermissions = tryOrNull { PosixFilePermissions.toString(potentialProblem.getPosixFilePermissions()) }
192+
if (posixPermissions != null) {
193+
append(", and POSIX permissions: $posixPermissions")
194+
}
195+
}
196+
}
197+
198+
return when (e) {
199+
is kotlin.io.AccessDeniedException -> kotlin.io.AccessDeniedException(e.file, e.other, message)
200+
is java.nio.file.AccessDeniedException -> java.nio.file.AccessDeniedException(e.file, e.otherFile, message)
201+
// should never happen
202+
else -> e
203+
}.also {
204+
it.stackTrace = e.stackTrace
205+
}
206+
}
207+
208+
private fun<T> tryOrLogShortException(log: Logger, block: () -> T) = try {
209+
block()
210+
} catch (e: Exception) {
211+
log.warn { "${e::class.simpleName}: ${e.message}" }
212+
null
213+
}

‎jetbrains-core/src/software/aws/toolkits/jetbrains/core/credentials/CredentialWriter.kt‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import com.intellij.openapi.ui.Messages
1818
import com.intellij.openapi.vfs.LocalFileSystem
1919
import org.jetbrains.annotations.TestOnly
2020
import software.amazon.awssdk.profiles.ProfileFileLocation
21+
import software.aws.toolkits.core.utils.createParentDirectories
22+
import software.aws.toolkits.core.utils.getLogger
23+
import software.aws.toolkits.core.utils.touch
24+
import software.aws.toolkits.core.utils.tryDirOp
25+
import software.aws.toolkits.core.utils.tryFileOp
26+
import software.aws.toolkits.core.utils.writeText
2127
import software.aws.toolkits.resources.message
2228
import software.aws.toolkits.telemetry.AwsTelemetry
2329
import java.io.File
@@ -128,23 +134,14 @@ object DefaultConfigFileWriter : ConfigFileWriter {
128134
""".trimIndent()
129135

130136
override fun createFile(file: File) {
131-
val parent = file.parentFile
132-
if (parent.mkdirs()) {
133-
parent.setReadable(false, false)
134-
parent.setReadable(true)
135-
parent.setWritable(false, false)
136-
parent.setWritable(true)
137-
parent.setExecutable(false, false)
138-
parent.setExecutable(true)
139-
}
140-
141-
file.writeText(TEMPLATE)
137+
val path = file.toPath()
138+
path.tryDirOp(LOG) { createParentDirectories() }
142139

143-
file.setReadable(false, false)
144-
file.setReadable(true)
145-
file.setWritable(false, false)
146-
file.setWritable(true)
147-
file.setExecutable(false, false)
148-
file.setExecutable(false)
140+
path.tryFileOp(LOG) {
141+
touch(restrictToOwner = true)
142+
writeText(TEMPLATE)
143+
}
149144
}
145+
146+
private val LOG = getLogger<DefaultConfigFileWriter>()
150147
}

0 commit comments

Comments
 (0)