Skip to content

Commit

Permalink
STONEBLD-1955 store all contaminate information
Browse files Browse the repository at this point in the history
  • Loading branch information
stuartwdouglas committed Nov 25, 2023
1 parent 7d5b7e1 commit e1b9ff1
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 44 deletions.
8 changes: 8 additions & 0 deletions deploy/crds/base/jvmbuildservice.io_dependencybuilds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,20 @@ spec:
contaminates:
items:
properties:
allowed:
type: boolean
buildId:
type: string
contaminatedArtifacts:
items:
type: string
type: array
gav:
type: string
rebuildAvailable:
type: boolean
source:
type: string
type: object
type: array
deployedArtifacts:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class DeployCommand implements Runnable {
private static final String DOT = ".";
private static final Set<String> ALLOWED_CONTAMINANTS = Set.of("-tests.jar");
public static final String IMAGE_DIGEST_OUTPUT = "Image Digest: ";
public static final String BUILD_ID = "build-id";
final BeanManager beanManager;
final ResultsUpdater resultsUpdater;

Expand Down Expand Up @@ -166,9 +167,7 @@ public void run() {

Set<String> gavs = new HashSet<>();
Map<String, Set<String>> contaminatedPaths = new HashMap<>();
Map<String, Set<String>> contaminatedGavs = new HashMap<>();
Map<String, Set<String>> allowedContaminatedPaths = new HashMap<>();
Map<String, Set<String>> allowedContaminatedGavs = new HashMap<>();
Map<String, Contaminates> contaminatedGavs = new HashMap<>();
// Represents directories that should not be deployed i.e. if a single artifact (barring test jars) is
// contaminated then none of the artifacts will be deployed.
Set<Path> toRemove = new HashSet<>();
Expand All @@ -184,25 +183,35 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
//we check every file as we also want to catch .tar.gz etc
var info = ClassFileTracker.readTrackingDataFromFile(Files.newInputStream(file), name);
for (var i : info) {
if (!allowedSources.contains(i.source)) {
Log.errorf("%s was contaminated by %s from %s", name, i.gav, i.source);
if (ALLOWED_CONTAMINANTS.stream().noneMatch(a -> file.getFileName().toString().endsWith(a))) {
gav.ifPresent(g -> contaminatedGavs.computeIfAbsent(i.gav, s -> new HashSet<>())
.add(g.getGroupId() + ":" + g.getArtifactId() + ":" + g.getVersion()));
int index = name.lastIndexOf("/");
Log.errorf("%s was contaminated by %s from %s", name, i.gav, i.source);
if (ALLOWED_CONTAMINANTS.stream().noneMatch(a -> file.getFileName().toString().endsWith(a))) {
int index = name.lastIndexOf("/");
boolean allowed = allowedSources.contains(i.source);
if (!allowed) {
if (index != -1) {
contaminatedPaths.computeIfAbsent(name.substring(0, index),
s -> new HashSet<>()).add(i.gav);
} else {
contaminatedPaths.computeIfAbsent("", s -> new HashSet<>()).add(i.gav);
}
toRemove.add(file.getParent());
} else {
Log.debugf("Ignoring contaminant for %s", file.getFileName());
}
} else {
gav.ifPresent(g -> contaminatedGavs.computeIfAbsent(i.gav, s -> {
Contaminates contaminates = new Contaminates();
contaminates.setGav(i.gav);
contaminates.setAllowed(allowed);
contaminates.setSource(i.source);
contaminates.setBuildId(i.getAttributes().get(BUILD_ID));
contaminates.setContaminatedArtifacts(new ArrayList<>());
return contaminates;
})
.getContaminatedArtifacts()
.add(g.getGroupId() + ":" + g.getArtifactId() + ":" + g.getVersion()));

} else {
Log.debugf("Ignoring contaminant for %s", file.getFileName());
}

}
if (gav.isPresent()) {
//now add our own tracking data
Expand All @@ -225,7 +234,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
+ gav.getVersion(),
"rebuilt",
Map.of("scm-uri", scmUri, "scm-commit", commit, "hermetic",
Boolean.toString(hermetic), "build-id", buildId)),
Boolean.toString(hermetic), BUILD_ID, buildId)),
Files.newOutputStream(temp), false);
Files.delete(file);
Files.move(temp, file);
Expand Down Expand Up @@ -269,14 +278,12 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
});
throw new RuntimeException("deploy failed");
}
//we still deploy, but without the contaminates
// This means the build failed to produce any deployable output.
// If everything is contaminated we still need the task to succeed so we can resolve the contamination.
for (var i : contaminatedGavs.entrySet()) {
gavs.removeAll(i.getValue());
if (!i.getValue().getAllowed()) {
gavs.removeAll(i.getValue().getContaminatedArtifacts());
}
}
generateBuildSbom();

if (isNotEmpty(mvnRepo) && mvnPassword.isEmpty()) {
Log.infof("Maven repository specified as %s and no password specified", mvnRepo);
URL url = new URL(mvnRepo);
Expand Down Expand Up @@ -304,6 +311,10 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
.getAuthorizationToken());
}
}

//we still deploy, but without the contaminates
// This means the build failed to produce any deployable output.
// If everything is contaminated we still need the task to succeed so we can resolve the contamination.
if (!gavs.isEmpty()) {
try {
cleanBrokenSymlinks(sourcePath);
Expand All @@ -323,10 +334,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {

List<Contaminates> newContaminates = new ArrayList<>();
for (var i : contaminatedGavs.entrySet()) {
Contaminates contaminates = new Contaminates();
contaminates.setContaminatedArtifacts(new ArrayList<>(i.getValue()));
contaminates.setGav(i.getKey());
newContaminates.add(contaminates);
newContaminates.add(i.getValue());
}
String serialisedContaminants = ResultsUpdater.MAPPER.writeValueAsString(newContaminates);
Log.infof("Updating results %s with contaminants %s and deployed resources %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ quarkus.quinoa.build-dir=dist
quarkus.quinoa.enable-spa-routing=true

quarkus.resteasy-reactive.path=/api/
%dev.sbom-discovery.enabled=false
%dev.sbom-discovery.enabled=true
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,20 @@ spec:
contaminates:
items:
properties:
allowed:
type: boolean
buildId:
type: string
contaminatedArtifacts:
items:
type: string
type: array
gav:
type: string
rebuildAvailable:
type: boolean
source:
type: string
type: object
type: array
deployedArtifacts:
Expand Down
16 changes: 15 additions & 1 deletion pkg/apis/jvmbuildservice/v1alpha1/dependencybuild_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type DependencyBuildStatus struct {
Conditions []metav1.Condition `json:"conditions,omitempty"`
State string `json:"state,omitempty"`
Message string `json:"message,omitempty"`
Contaminants []Contaminant `json:"contaminates,omitempty"`
Contaminants []*Contaminant `json:"contaminates,omitempty"`
// PotentialBuildRecipes additional recipes to try if the current recipe fails
PotentialBuildRecipes []*BuildRecipe `json:"potentialBuildRecipes,omitempty"`
CommitTime int64 `json:"commitTime,omitempty"`
Expand Down Expand Up @@ -115,6 +115,16 @@ func (r *DependencyBuildStatus) CurrentBuildAttempt() *BuildAttempt {
return r.BuildAttempts[len(r.BuildAttempts)-1]
}

func (r *DependencyBuildStatus) ProblemContaminates() []*Contaminant {
problemContaminates := []*Contaminant{}
for _, i := range r.Contaminants {
if !i.Allowed {
problemContaminates = append(problemContaminates, i)
}
}
return problemContaminates
}

type BuildRecipe struct {
//Deprecated
Pipeline string `json:"pipeline,omitempty"`
Expand All @@ -136,6 +146,10 @@ type BuildRecipe struct {
type Contaminant struct {
GAV string `json:"gav,omitempty"`
ContaminatedArtifacts []string `json:"contaminatedArtifacts,omitempty"`
BuildId string `json:"buildId,omitempty"`
Source string `json:"source,omitempty"`
Allowed bool `json:"allowed,omitempty"`
RebuildAvailable bool `json:"rebuildAvailable,omitempty"`
}
type AdditionalDownload struct {
Uri string `json:"uri,omitempty"`
Expand Down
8 changes: 6 additions & 2 deletions pkg/apis/jvmbuildservice/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 9 additions & 7 deletions pkg/reconciler/artifactbuild/artifactbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,15 +383,17 @@ func (r *ReconcileArtifactBuild) handleStateComplete(ctx context.Context, log lo
if db.Status.State != v1alpha1.DependencyBuildStateContaminated {
continue
}
var newContaminates []v1alpha1.Contaminant
for _, contaminant := range db.Status.Contaminants {
if contaminant.GAV != abr.Spec.GAV {
newContaminates = append(newContaminates, contaminant)
allOk := true
for i := range db.Status.Contaminants {
contaminant := db.Status.Contaminants[i]
if contaminant.GAV == abr.Spec.GAV {
contaminant.RebuildAvailable = true
} else if !contaminant.Allowed && !contaminant.RebuildAvailable {
allOk = false
}
}
log.Info("Attempting to resolve contamination for dependencybuild", "dependencybuild", db.Name+"-"+db.Spec.ScmInfo.SCMURL+"-"+db.Spec.ScmInfo.Tag, "old", db.Status.Contaminants, "new", newContaminates)
db.Status.Contaminants = newContaminates
if len(db.Status.Contaminants) == 0 {
if allOk {
log.Info("Attempting to resolve contamination for dependencybuild as all contaminates are ready", "dependencybuild", db.Name+"-"+db.Spec.ScmInfo.SCMURL+"-"+db.Spec.ScmInfo.Tag)
//TODO: we could have a situation where there are still some contamination, but not for artifacts that we care about
//kick off the build again
log.Info("Contamination resolved, moving to state new", "dependencybuild", db.Name+"-"+db.Spec.ScmInfo.SCMURL+"-"+db.Spec.ScmInfo.Tag)
Expand Down
12 changes: 9 additions & 3 deletions pkg/reconciler/artifactbuild/artifactbuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func TestStateBuilding(t *testing.T) {
Labels: map[string]string{DependencyBuildIdLabel: util.HashString("")},
},
Spec: v1alpha1.DependencyBuildSpec{},
Status: v1alpha1.DependencyBuildStatus{State: v1alpha1.DependencyBuildStateContaminated, Contaminants: []v1alpha1.Contaminant{{GAV: "com.test:test:1.0", ContaminatedArtifacts: []string{"a:b:1"}}}},
Status: v1alpha1.DependencyBuildStatus{State: v1alpha1.DependencyBuildStateContaminated, Contaminants: []*v1alpha1.Contaminant{{GAV: "com.test:test:1.0", ContaminatedArtifacts: []string{"a:b:1"}}}},
}
g.Expect(controllerutil.SetOwnerReference(abr, db, reconciler.scheme))
g.Expect(client.Create(ctx, db))
Expand Down Expand Up @@ -357,7 +357,7 @@ func TestStateCompleteFixingContamination(t *testing.T) {
Labels: map[string]string{DependencyBuildIdLabel: util.HashString("")},
},
Spec: v1alpha1.DependencyBuildSpec{},
Status: v1alpha1.DependencyBuildStatus{State: v1alpha1.DependencyBuildStateContaminated, Contaminants: []v1alpha1.Contaminant{{GAV: "com.test:test:1.0", ContaminatedArtifacts: []string{"a:b:1"}}}},
Status: v1alpha1.DependencyBuildStatus{State: v1alpha1.DependencyBuildStateContaminated, Contaminants: []*v1alpha1.Contaminant{{GAV: "com.test:test:1.0", ContaminatedArtifacts: []string{"a:b:1"}}}},
}
client, reconciler = setupClientAndReconciler(abr, contaiminated)
}
Expand All @@ -367,6 +367,12 @@ func TestStateCompleteFixingContamination(t *testing.T) {
g.Expect(reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: metav1.NamespaceDefault, Name: "test"}}))
db := v1alpha1.DependencyBuild{}
g.Expect(client.Get(ctx, types.NamespacedName{Namespace: metav1.NamespaceDefault, Name: contaminatedName}, &db))
g.Expect(db.Status.Contaminants).Should(BeEmpty())
allOk := true
for _, i := range db.Status.Contaminants {
if !i.Allowed && !i.RebuildAvailable {
allOk = false
}
}
g.Expect(allOk).Should(BeTrue())
})
}
22 changes: 15 additions & 7 deletions pkg/reconciler/dependencybuild/dependencybuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ func (r *ReconcileDependencyBuild) handleBuildPipelineRunReceived(ctx context.Co
digest = i.Value.StringVal
} else if i.Name == artifactbuild.PipelineResultContaminants {

db.Status.Contaminants = []v1alpha1.Contaminant{}
db.Status.Contaminants = []*v1alpha1.Contaminant{}
//unmarshal directly into the contaminants field
err := json.Unmarshal([]byte(i.Value.StringVal), &db.Status.Contaminants)
if err != nil {
Expand Down Expand Up @@ -689,7 +689,7 @@ func (r *ReconcileDependencyBuild) handleBuildPipelineRunReceived(ctx context.Co
for _, i := range pr.Status.Results {
if i.Name == artifactbuild.PipelineResultContaminants {

db.Status.Contaminants = []v1alpha1.Contaminant{}
db.Status.Contaminants = []*v1alpha1.Contaminant{}
//unmarshal directly into the contaminants field
err := json.Unmarshal([]byte(i.Value.StringVal), &db.Status.Contaminants)
if err != nil {
Expand All @@ -712,15 +712,16 @@ func (r *ReconcileDependencyBuild) handleBuildPipelineRunReceived(ctx context.Co
}
}

if len(db.Status.Contaminants) == 0 {
problemContaminates := db.Status.ProblemContaminates()
if len(problemContaminates) == 0 {
db.Status.State = v1alpha1.DependencyBuildStateComplete
} else {
r.eventRecorder.Eventf(db, v1.EventTypeWarning, "BuildContaminated", "The DependencyBuild %s/%s was contaminated with community dependencies", db.Namespace, db.Name)
//the dependency was contaminated with community deps
//most likely shaded in
//we don't need to update the status here, it will be handled by the handleStateComplete method
//even though there are contaminates they may not be in artifacts we care about
err := r.handleBuildCompletedWithContaminants(ctx, db, log)
err := r.handleBuildCompletedWithContaminants(ctx, db, log, problemContaminates)
if err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -815,7 +816,7 @@ func (r *ReconcileDependencyBuild) dependencyBuildForPipelineRun(ctx context.Con
// no actual request for these artifacts. This can change if new artifacts are requested, so even when complete
// we still need to verify that hte build is ok
// this method will always update the status if it does not return an error
func (r *ReconcileDependencyBuild) handleBuildCompletedWithContaminants(ctx context.Context, db *v1alpha1.DependencyBuild, l logr.Logger) error {
func (r *ReconcileDependencyBuild) handleBuildCompletedWithContaminants(ctx context.Context, db *v1alpha1.DependencyBuild, l logr.Logger, problemContaminates []*v1alpha1.Contaminant) error {

ownerGavs := map[string]bool{}
db.Status.State = v1alpha1.DependencyBuildStateComplete
Expand All @@ -838,7 +839,7 @@ func (r *ReconcileDependencyBuild) handleBuildCompletedWithContaminants(ctx cont
ownerGavs[ab.Spec.GAV] = true
}
}
for _, contaminant := range db.Status.Contaminants {
for _, contaminant := range problemContaminates {
for _, artifact := range contaminant.ContaminatedArtifacts {
if ownerGavs[artifact] {
db.Status.State = v1alpha1.DependencyBuildStateContaminated
Expand Down Expand Up @@ -883,7 +884,14 @@ func (r *ReconcileDependencyBuild) handleBuildCompletedWithContaminants(ctx cont
}
func (r *ReconcileDependencyBuild) handleStateContaminated(ctx context.Context, db *v1alpha1.DependencyBuild) (reconcile.Result, error) {
contaminants := db.Status.Contaminants
if len(contaminants) == 0 {
allOk := true
for _, i := range contaminants {
if !i.Allowed && !i.RebuildAvailable {
allOk = false
break
}
}
if allOk {
//all fixed, just set the state back to building and try again
//this is triggered when contaminants are removed by the ABR controller
//setting it back to building should re-try the recipe that actually worked
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/dependencybuild/dependencybuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ func TestStateBuilding(t *testing.T) {
g.Expect(client.Update(ctx, pr)).Should(BeNil())
db := getBuild(client, g)
g.Expect(controllerutil.SetOwnerReference(&ab, db, reconciler.scheme)).Should(BeNil())
db.Status.Contaminants = []v1alpha1.Contaminant{{GAV: "com.acme:foo:1.0", ContaminatedArtifacts: []string{TestArtifact}}}
db.Status.Contaminants = []*v1alpha1.Contaminant{{GAV: "com.acme:foo:1.0", ContaminatedArtifacts: []string{TestArtifact}}}
g.Expect(client.Update(ctx, db))
g.Expect(reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: taskRunName}))
g.Expect(reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: buildName}))
Expand Down

0 comments on commit e1b9ff1

Please sign in to comment.